CHE-5339: Mark with color Git changed files in project explorer #5722

vinokurig · 2017-07-18T11:54:45Z

What does this PR do?

Mark with colour Git changed files in project explorer and editor's tabs:

What issues does this PR fix or reference?

#5339

Changelog

Colors project explorer files and editor tabs according to their git change state.

Release Notes

Colors project explorer files and editor tabs according to their git change state.

Docs PR

eclipse-che/che-docs#275

vzhukovs · 2017-07-18T12:09:14Z

ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/vcs/HasVcsStatus.java

+package org.eclipse.che.ide.api.vcs;
+
+/**
+ * Indicates that specified node has VCS status attribute.


specified node -> specified resource

and so on on the below javadoc

vzhukovs · 2017-07-18T12:13:08Z

...che-core-ide-app/src/main/java/org/eclipse/che/ide/part/editor/EditorPartStackPresenter.java

@@ -211,6 +219,14 @@ public void addPart(@NotNull PartPresenter part) {

        final EditorTab editorTab = tabItemFactory.createEditorPartButton(editorPart, this);

+        resourceManagerFactory.newResourceManager(appContext.getDevMachine())


Instead of resourceManagerFactory.newResourceManager(appContext.getDevMachine()).getFile(file.getLocation()) use appContext.getWorkspaceRoot().getFile(file.getLocation())

sunix · 2017-07-18T13:09:30Z

_ I don't think the colors chosen are good: red for unstaged looks like there is an error
_ Red is default color for unstaged files in console git
_ in a console it's different, in the IDE, if you show something in red, i would open the file and try to find where is the error

vinokurig · 2017-07-18T13:15:58Z

@sunix How about yellow color for unstaged files?

slemeur · 2017-07-18T13:20:16Z

Please adjust the colors to the one in the requirements https://cloud.githubusercontent.com/assets/1636769/26213265/b13531d4-3bf8-11e7-9516-8217c68a6af8.png

It was yellow in the requirements.

slemeur · 2017-07-18T13:22:09Z

Missing docs update.

slemeur

Colors need to be changed
Documentation PR needs to be provided

vinokurig · 2017-07-18T13:26:51Z

@slemeur I don't se a color for untracked files in your mockup, Is it OK to have next colors:
new: green
modified: blue
untracked: yellow

Documentation PR needs to be provided

In progress 🙂

voievodin · 2017-07-18T13:27:17Z

ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/parts/EditorTab.java

+     * @param color
+     *         color to set
+     */
+    void setTitleColor(String color);


Do you think it makes sense to specify the format of the color in the javadoc, like hash #000000 or color name like blue ?

Yep. Any css valid color.

codenvy-ci · 2017-07-18T13:37:42Z

Build # 3092 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3092/ to view the results.

vzhukovs · 2017-07-18T13:38:56Z

@slemeur what about these colors:

added: #0a7700
modified: #0032a0
untracked: #993300

?

vparfonov · 2017-07-18T13:41:11Z

...e-api/src/main/java/org/eclipse/che/ide/api/event/ng/JsonRpcWebSocketAgentEventListener.java

+    private void initializeGitChangeWatcher() {
+        requestTransmitter.newRequest()
+                          .endpointId("ws-agent")
+                          .methodName("track:git-change")


please, rename method to the "track/git-change" (this is our internal agreement)

vparfonov · 2017-07-18T13:41:49Z

...e-api/src/main/java/org/eclipse/che/ide/api/event/ng/JsonRpcWebSocketAgentEventListener.java

+    private void initializeGitIndexWatcher() {
+        requestTransmitter.newRequest()
+                          .endpointId("ws-agent")
+                          .methodName("track:git-index")


ename method to the "track/git-index"

vinokurig · 2017-07-18T13:48:21Z

@slemeur I will prepare a docs PR when we will choose file colours to have actual screenshots

codenvy-ci · 2017-07-18T14:46:43Z

Build # 3094 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3094/ to view the results.

codenvy-ci · 2017-07-19T07:32:56Z

Build # 3101 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3101/ to view the results.

voievodin · 2017-07-19T07:34:45Z

ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/vcs/HasVcsStatus.java

+package org.eclipse.che.ide.api.vcs;
+
+/**
+ * Indicates that specified resource has VCS status attribute.


Can you please elaborate on what it means to have VCS status attribute?
Does it mean that each file has VCS status if the project is under VCS?

It means that the resource can store and return VCS status

voievodin · 2017-07-19T08:18:54Z

...e-plugin-git-ext-git/src/main/java/org/eclipse/che/ide/ext/git/client/GitChangesHandler.java

+                    .withBiConsumer(this::apply);
+    }
+
+    public void apply(String endpointId, GitChangeEventDto dto) {


Do you think it makes sense to decouple jsonrpc transport and GIT VCS status -> VCS status adaptation ?

In current step we implement file colors only for git, so I think it is not bad to keep VCS status convertation logic inside Git changes transmitter.
The problem is that Git status command returns object that has lists of files for each status, but SVN status request returns just CLI output, so it is not clear, how to build a common Status converter (what common parameter it should receive).
I think this issue can be solved while implementig file colors for SVN.

Thank you for the description, if @vparfonov is okay with it, let's leave it in current state.

slemeur · 2017-07-19T11:36:16Z

Colors:

added: #3CAC64
modified: #DCCB58
untracked: #9A5CFF

…E-5339

sunix · 2017-07-24T12:51:14Z

IMO we should reuse colors that exist and used already in the IDE:

green should be the same green you have in the "class" icon,
yellow should be the one used for "folder" icon and
blue the one used for the cube in project icon.

vzhukovs · 2017-07-24T13:50:34Z

@sunix +1. As for me(I have a protanopia), modified color is not readable.

vinokurig · 2017-07-24T14:13:25Z

@sunix +1

But I would set more darker yellow in the light theme. @slemeur WDYT?

bmicklea · 2017-07-24T16:43:13Z

+1 on a darker yellow for light theme.

slemeur · 2017-08-08T11:36:39Z

The colours for the dark theme are great.
I would also darker colors for the light theme - especially the yellow (you could try #BE9800).

vinokurig · 2017-08-09T13:01:15Z

@slemeur
new colors for light theme:
added: #0a7700
modified: #0768B0
untracked: #ab8900

vinokurig · 2017-08-09T13:21:48Z

@slemeur We need to decide what to do with colorizing editor tabs. My proposal is to change colorizing editor tabs caused by java error/warning to colored wavy underlinings:

WDYT?

slemeur · 2017-08-09T15:01:44Z

Ok for the colors for the light theme.

Your proposal for showing error/warning is fine - you'll differentiate the error/warning state with the wavy underlines' color?

vinokurig · 2017-08-10T08:33:14Z

@slemeur

you'll differentiate the error/warning state with the wavy underlines' color?

Yes
error:

warning:

codenvy-ci · 2017-08-10T09:28:32Z

Build # 3327 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3327/ to view the results.

codenvy-ci · 2017-08-10T14:42:01Z

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/3333/

codenvy-ci · 2017-08-11T15:58:04Z

Build # 3351 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3351/ to view the results.

codenvy-ci · 2017-08-14T05:40:43Z

Build # 3357 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3357/ to view the results.

…pse-che#5722)

CHE-5339: Mark with colour Git changed files in project explorer

146e559

vinokurig requested review from vparfonov, vzhukovs, voievodin and bmicklea July 18, 2017 11:54

vzhukovs reviewed Jul 18, 2017

View reviewed changes

vzhukovs approved these changes Jul 18, 2017

View reviewed changes

slemeur self-requested a review July 18, 2017 13:21

slemeur suggested changes Jul 18, 2017

View reviewed changes

voievodin reviewed Jul 18, 2017

View reviewed changes

fixup! CHE-5339: Mark with colour Git changed files in project explorer

19a68b9

vparfonov reviewed Jul 18, 2017

View reviewed changes

Merge branch 'master' of github.com:eclipse/che into CHE-5339

176eeba

fixup! Merge branch 'master' of github.com:eclipse/che into CHE-5339

a38f0c7

voievodin reviewed Jul 19, 2017

View reviewed changes

Igor Vinokur added 2 commits July 19, 2017 16:17

fixup! fixup! Merge branch 'master' of github.com:eclipse/che into CH…

c5e0a01

…E-5339

Merge branch 'master' of github.com:eclipse/che into CHE-5339

752ff4e

Merge branch 'master' of github.com:eclipse/che into CHE-5339

51774dd

bmicklea approved these changes Aug 9, 2017

View reviewed changes

Merge branch 'master' of github.com:eclipse/che into CHE-5339

0cc1574

fixup! CHE-5339: Mark with colour Git changed files in project explorer

c192c6d

slemeur approved these changes Aug 10, 2017

View reviewed changes

vinokurig mentioned this pull request Aug 10, 2017

CHE-5339: Mark with colour Git changed files in project explorer eclipse-che/che-docs#275

Merged

fixup! CHE-5339: Mark with colour Git changed files in project explorer

fe0820f

Igor Vinokur added 2 commits August 11, 2017 17:15

fixup! CHE-5339: Mark with colour Git changed files in project explorer

b1a6682

Merge branch 'master' of github.com:eclipse/che into CHE-5339

9c0069f

Merge with master

54726b1

vinokurig added this to the 5.17.0 milestone Aug 14, 2017

vinokurig merged commit 373e93d into master Aug 14, 2017

vinokurig deleted the CHE-5339 branch August 14, 2017 05:56

dmytro-ndp pushed a commit to dmytro-ndp/che that referenced this pull request Aug 16, 2017

CHE-5339: Mark with color Git changed files in project explorer (ecli…

646a26a

…pse-che#5722)

slemeur mentioned this pull request Aug 17, 2017

Wrong git status displayed in editor's tab after pushing changes #6033

Closed

vparfonov mentioned this pull request Sep 4, 2017

Workspaces stuck consuming 100% CPU since 5.17 #6114

Closed

tolusha mentioned this pull request Sep 19, 2017

Rework git status feature #6342

Closed

		@@ -211,6 +219,14 @@ public void addPart(@NotNull PartPresenter part) {

		final EditorTab editorTab = tabItemFactory.createEditorPartButton(editorPart, this);

		resourceManagerFactory.newResourceManager(appContext.getDevMachine())

CHE-5339: Mark with color Git changed files in project explorer #5722

CHE-5339: Mark with color Git changed files in project explorer #5722

Conversation

vinokurig commented Jul 18, 2017 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Changelog

Release Notes

Docs PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunix commented Jul 18, 2017 • edited Loading

vinokurig commented Jul 18, 2017

slemeur commented Jul 18, 2017

slemeur commented Jul 18, 2017

slemeur left a comment

Choose a reason for hiding this comment

vinokurig commented Jul 18, 2017

voievodin Jul 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codenvy-ci commented Jul 18, 2017

vzhukovs commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinokurig commented Jul 18, 2017 • edited Loading

codenvy-ci commented Jul 18, 2017

codenvy-ci commented Jul 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voievodin Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slemeur commented Jul 19, 2017

sunix commented Jul 24, 2017 • edited Loading

vzhukovs commented Jul 24, 2017

vinokurig commented Jul 24, 2017 • edited Loading

bmicklea commented Jul 24, 2017

slemeur commented Aug 8, 2017

vinokurig commented Aug 9, 2017

vinokurig commented Aug 9, 2017

slemeur commented Aug 9, 2017

vinokurig commented Aug 10, 2017

codenvy-ci commented Aug 10, 2017

codenvy-ci commented Aug 10, 2017

codenvy-ci commented Aug 11, 2017

codenvy-ci commented Aug 14, 2017

vinokurig commented Jul 18, 2017 •

edited

Loading

sunix commented Jul 18, 2017 •

edited

Loading

voievodin Jul 18, 2017 •

edited

Loading

vinokurig commented Jul 18, 2017 •

edited

Loading

voievodin Jul 19, 2017 •

edited

Loading

sunix commented Jul 24, 2017 •

edited

Loading

vinokurig commented Jul 24, 2017 •

edited

Loading