PR: Color code files by version control status in Project Explorer

[alexschw]

Description of Changes

• Wrote at least one-line docstrings (for any new functions)
• Included a screenshot (if affecting the UI)
• Color code should not be hardcoded
• Created, moved files should be colored (even those added from outside spyder)
• Look in the sub-directories for git repos too
• Added unit test(s) covering the changes (if testable)
• Make color settings modifiable in UI

TODO (Future PR)

• Support of other vcs features (add, remove ...)

I added a color code for the project explorer that is responding to the version control system(vcs) and colors files accordingly. It is a feature similar to VSCode, PyCharm and similar editors. Next step would be a simple solution to add and remove files without any external sources (maybe I will do this in another PR).

Added = green
Changed = blue
Untracked = red
Ignored = grey

Sources

https://stackoverflow.com/a/40455027

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: alexschw

[pep8speaks]

Hello @alexschw! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-01-06 18:25:25 UTC

[ccordoba12]

This looks really good, thanks!

A very quick review: please revert all blank space removal you performed because that could impact the work of others or mine too when merging between branches. Thanks!

[CAM-Gerlach]

Thanks for your contribution! This would be a really great enhancement to Spyder and I know a lot of users would appreciate improved VCS functionality; its probably one of our most-requested features that we aren't currently in the process of adding yet for Spyder 4,

Unfortunately, I could not actually get this feature to work as shown in your screenshot (I wouldn't have normally done a full review on a WiP PR, but I figured I'd try to replicate your screenshot at least since it was apparently already working). I tested it on multiple projects, all with a "clean" configuration (--safe-mode), both with staged, unstaged and untracked changes both before and after I'd opened the project, and with the project directory being either the repo root or one directory up, but I didn't see any indication in the Project Explorer like your screenshot shows even when started with clean settings (--safe-mode). Instead, I got an error on opening projects, which differed between whether I was opening a project with a project directory inside or outside the repo root directory.

On opening a project where the project dir wasn't the repo root (or without a repo at all), my saved session didn't load in the Editor (which stayed completely blank with nothing open) and I got the following error:

  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\projects\plugin.py", line 147, in <lambda>
    lambda v: self.main.editor.setup_open_files())
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\editor\plugin.py", line 2932, in setup_open_files
    is_vertical, cfname, clines = layout.get('splitsettings')[0]
AttributeError: 'NoneType' object has no attribute 'get'

However, the project did open in the Project Explorer, and I was able to manually open files from there. If I didn't open any files, the error would also appear on closing the project.

Any project with the project directory being the root of the repo resulted in the following error on opening:

  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\projects\plugin.py", line 147, in <lambda>
    lambda v: self.main.editor.setup_open_files())
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\editor\plugin.py", line 2932, in setup_open_files
    is_vertical, cfname, clines = layout.get('splitsettings')[0]
AttributeError: 'NoneType' object has no attribute 'get'
Traceback (most recent call last):
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\projects\plugin.py", line 441, in update_explorer
    self.explorer.setup_project(self.get_active_project_path())
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\projects\widgets\explorer.py", line 232, in setup_project
    self.set_project_dir(directory)
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\projects\widgets\explorer.py", line 213, in set_project_dir
    self.treewidget.set_folder_names([project])
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\explorer\widgets.py", line 1067, in set_folder_names
    self.set_vcs_state(path_list[0])
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\plugins\explorer\widgets.py", line 1057, in set_vcs_state
    status_dict = vcs.get_vcs_status(folderPath)
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\spyder\spyder\utils\vcs.py", line 121, in get_vcs_status
    for fString in (x for x in out[:-1].split("\n") if x):
TypeError: a bytes-like object is required, not 'str'

however, the project explorer loaded my project directory, although my session was not loaded (like with other projects).

And of course, nothing showed up in terms of added/changed/etc files in the Project Explorer:

Also, the first time I attempted to open a Spyder project on this branch (while currently not having one open), one previously created and used successfully with Spyder 3 in Spyder 4 for testing; its only semi-unusual feature (to my knowledge) being that it was on a non-startup partition of my boot drive. However, Spyder stopped responding and hung indefinitely after clicking "Open" in the file chooster dialog, and following that would not launch (it would just hang on a blank window with no UI) unless settings were reset, since the project was loaded automatically on startup. However, I'm not sure if its related this PR, since I couldn't repro it on the same project in --safe-mode/clean config (though I did get other errors specific to this PR), and it persisted after I changed branches back to master.

Thanks!

[alexschw]

Thank you for the thorough review, @CAM-Gerlach.

I am very sorry, that it crashed your Projects. I could reproduce the same error on a freshly pulled master repo. I think this #8375 issue is related.

And of course, nothing showed up in terms of added/changed/etc files in the Project Explorer

I could not yet reproduce that yet. Maybe it is due to MINGW instead of the linux git, otherwise it may be that the directory is not correctly split due to spaces. I will get a Windows and look into that as soon as the previous problem is solved.

[CAM-Gerlach]

Thanks for your response @alexschw !

I could reproduce the same error on a freshly pulled master repo.

The not being able to load the saved session or the actual hang/crash? Also, I did test on master and didn't reproduce either of the errors nor not loading the save session, but now that I try again I can repro the not loading saved state, although I can't confirm if it happens on projects that I haven't already loaded/closed with your PR branch active. However, I don't get either of the errors loading projects either during my previous testing or now (nor the crash, but I couldn't repro that consistently in either) so those are apparently indeed due to this PR.

Maybe it is due to MINGW instead of the linux git

I don't think so, since I just tested on my Linux VM (Fedora 28) and I got the exact same error on your PR branch after creating and opening a fresh VCS-tracked project and I wasn't able to get files to show up as added, modified, etc just like on Windows.

otherwise it may be that the directory is not correctly split due to spaces.

Some of the directories I tried are on an absolute path that has spaces, while others are not, and I reprod one error or the other consistently on both,

[CAM-Gerlach]

@alexschw It crashes? Or just raises the above exception?

I can now repro it in master as well (I couldn't before); my naive guess is that it may be due to a side effect of #8375 , or at least be exposed by it. It's not related to this PR, so I'll investigate further and open a new issue. Thanks!

[CAM-Gerlach]

@alexschw Any progress reproing the non-functional colors? I could on both my Linux VM and my Windows machine, so I don't think its platform specific. This would be a great improvement; we just need to make sure it works reliably. Maybe if you provided some details on your environment, I could try to repro it working or vice versa?

Thanks!

[alexschw]

@CAM-Gerlach I think this solves the problem.
If it does work, I will work on unit-tests.
If it doesn't, my environment is a Windows 10
PyQt5 5.11.3
QtPy 1.5.2
pygments 2.3.0
qdarkstyle 2.6.4
sphinx 1.8.2
pyls 0.21.4
nbconvert 5.4.0
qtconsole 4.4.3
IPython 7.2.0
pylint 2.2.2

[CAM-Gerlach]

Okay, it mostly works now, thanks! However, there are some remaining issues, which may or may not be easily fixable (if not, then we can merge without them for now):

• It only works if the project directory is the same as the repo root, whereas many times at least in my workflow the project dir is the directory above the repo root dir, and thus the highlighting does not function for most of my projects. Can you check for Git repos at least one level below the root dir (and maybe even in the parent directory as well)?
• Untracked files do not show up as red (and are merely the normal color) if they are created, moved etc. either within or outside of Spyder, only once changes are first saved within Spyder.

Also, see the individual comments on your code. Looking forward to your tests as well! Let us know if you need guidance on that.

[alexschw]

Looking forward to your tests as well! Let us know if you need guidance on that.

I tried to work on the tests, but if I'm trying to run the current tests (python3, pytest, pytest-cov and pytest-qt installed) I get Errors saying fixture 'mocker' not found...
I think I need guidance... :)

[jnsebgosselin]

You need to install pytest-mock

https://github.com/pytest-dev/pytest-mock

[alexschw]

Okay. I hope the tests run through now.
Could you review it again please @CAM-Gerlach?

[CAM-Gerlach]

Thanks for your latest changes. I tested this, and it indeed works most of the time on projects in immediate subdirectories. However, there are still some problems:

• Ignored directories (as opposed to files) are shown in the normal color, and ignored files, even those with an ignored extension (that works elsewhere) and newly added and saved within Spyder in that directory are located in an ignored directory, also show up in the normal color.
• At least on my machine, staged files no longer show up in green properly. When doing git add on an existing modified file across many tests, the file continues showing its old color, even after triggering a save of another file.
• Furthermore, if additional modifications are then made to the file and it is saved, it turns the standard (white) color, even if it is not tracked, ignored, modified, etc.

Most critically, It adds a pretty big performance hit to Spyder when an opened project had a fair number (10+) top-level subdirectories with Git repos, at least when tested on an i7-3730QM 4 core/ 8 thread laptop with 24 GB DDR3-1600 and a high-end SATA 850 EVO SSD which is particularly critical for us since one of the advantages users frequently cite about Spyder is it being a relatively lightweight IDE with snappy performance. For example (all of these operations were essentially instant in Spyder previously regardless of project size, or when working :

• First loading such a project took about 10 seconds, with the whole Spyder UI locking up and not responding during that time. Subsequent loads took only a couple of seconds like before.
• Anytime I saved a file in such a project, the entire UI froze for nearly a full second (this was probably the most serious and long-term annoying problem)
• Clicking the dropdown arrow of any directory, subdirectory etc. in the Project Explorer resulted in a ~2 second delay before anything happened the first time that sub/directory was opened; subsequently clicking it again displayed and hid it instantly.

Is there anything you can do about these performance hits? Having to deal with this much lag, especially when saving, would make me hesitate to use Spyder projects in these situations. since I save very frequently.

Thanks!

[alexschw]

• Ignored directories (as opposed to files) are shown in the normal color, and ignored files, even those with an ignored extension (that works elsewhere) and newly added and saved within Spyder in that directory are located in an ignored directory, also show up in the normal color.

I also got that feeling, yet in my case it is just that there is such a little difference between #FFFFFF and the proposed #999999 color. So I suggest using something inbetween (like #777777)

• At least on my machine, staged files no longer show up in green properly. When doing git add on an existing modified file across many tests, the file continues showing its old color, even after triggering a save of another file.

• Furthermore, if additional modifications are then made to the file and it is saved, it turns the standard (white) color, even if it is not tracked, ignored, modified, etc.

yes, this is not working by now. I will work on that.

Is there anything you can do about these performance hits? Having to deal with this much lag, especially when saving, would make me hesitate to use Spyder projects in these situations. since I save very frequently.

I am not sure how to shorten the loading time when opening a project. I would either check if there are more than 5 subrepos and disable the checking or i could make the coloring a project option that you could disable by hand.
I have an idea, on how to speed up the saving. Will work on that. Good suggestion, thanks.

[CAM-Gerlach]

I also got that feeling, yet in my case it is just that there is such a little difference

Okay, how about we try #808080, 50% gray?

I am not sure how to shorten the loading time when opening a project.

This is probably acceptable if you can't reduce it, since it only seems to occur the first time a project is opened. It would be nice to have an option to disable the coloring in the Project Explorer options menu since some users may prefer to have the feature off, which would also disable the checking until it was turned back on.

It is the lag when saving and the clicking the directory dropdown arrows in the Project Explorer that is much more problematic, so hopefully you can greatly reduce both of those. Thanks!

[alexschw]

Could you review this again @CAM-Gerlach? Maybe it can still be included in spyder4.

[ccordoba12]

@alexschw, thanks a lot for your work but unfortunately this won't be included in Spyder 4 because we're in bugfixing mode now.

However, it'll be one of the first PRs to be merged for Spyder 5 (to be released in the next six to eight months).

[CAM-Gerlach]

@alexschw I'm no longer a Spyder core developer, so unfortunately I cannot, besides for the reasons @ccordoba12 explained.

@ccordoba12 Since I'd been wondering the same, do you have a rough idea at what point will you branch 4.x and master will mean Spyder 5, so this PR can be merged? As of the Spyder 4 final release, before, or after? Somewhat tangential, but will that also be the point at which new code will no longer need to be backward compatible with Python 2, you can drop Py2 CIs on master and we can help strip out the Python 2 cruft from the codebase?

[goanpeca]

As of the Spyder 4 final release, before, or after?

After

you can drop Py2 CIs on master and we can help strip out the Python 2 cruft from the codebase?

I will be handling the Python 3 migration. The plan is to update py3compat so that only python3 is used and any refactoring needed for the public facing API. The plan will not be to remove code as that takes time, effort and can lead to subtle bugs.

[CAM-Gerlach]

@goanpeca Replied to you on Gitter to avoid going too far off topic.

[alexschw]

@CAM-Gerlach Okay, thank you anyways.
@ccordoba12 Oh, damn I am a few days to late... ;) Could you assign another reviewer than?

[CAM-Gerlach]

@alexschw I'm not sure what you mean; I neither am assigned to this PR nor do I have a pending review for it. As you can see @dalthviz was assigned to complete this for Spyder 4, and either he, @goanpeca or @ccordoba12 can review it, all of whom are already subscribed to this PR.

[alexschw]

okay. I though a reviewer has to be requested somewhere. I just saw, that you are the only one listed at the top under reviewers. But that is just due to you having reviewed this PR in the past. Thank you for the clarification.

[ccordoba12]

@dalthviz, please give this one a review when you have some free time.

[alexschw]

I am not sure if this is needed anymore, as @trollodel has made a very good contribution. It fits and exceeds my vision of this PR. I would be fine with closing this.

[trollodel]

@alexschw I'm not sure what are the intentions about #13562, but if it will be merged at some point, IMO this PR is still valid. It requires a bit of rework to use the VCS API I made in #13562.

[trollodel]

@alexschw (and anyone interested) you may be interested in https://github.com/spyder-ide/spyder-vcs. Feel free to join us there.