Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the staging UI to make it easier for git beginners #448

Merged
merged 12 commits into from
Nov 22, 2019

Conversation

telamonian
Copy link
Member

This is a resumption of the work from #437 following the major refactor that was done in #438. I've cherry picked various lines from #437 to get this started. Here's the info from the old PR:

For people who haven't used any VCS before, git can prove to be somewhat overwhelming. In particular, the untracked/unstaged/staged concept can seem very foreign to someone who up until now has never done any kind of "commit" more complicated than saving a file to disk. For that reason I think it would be idea to have an option to switch the staging UI to a "simplified" mode. As a model, I'm thinking along the lines of the UI in the Github desktop app:

image

It would simply show which files have changed, rather than splitting them into categories. When the commit button is pushed, all changed files will be automatically staged and commited (although there should also be something like the Github app's checkboxes to opt files out of a commit).

Additionally, some color coding of file names would be nice, a la VSCode:

image

Currently this PR is at proof of concept stage. It adds a simpleStaging flag to the user settings. When enabled, the staging UI is simplified, and only shows a single "changed" category. When disabled, the staging UI reverts back to the standard version.

…r in jupyterlab#438

too much stuff has changed to make a rebase/merge worthwhile, so I've started a new branch and am cherry picking lines from the previous work in jupyterlab#437
@telamonian telamonian force-pushed the simple_staging_refactored branch from 676c4e2 to 4a87a37 Compare November 11, 2019 01:54
@telamonian
Copy link
Member Author

This is now fully functional and ready for review. Here's the current state of the simple staging UI:

git_simple_staging_animated

Now, with the "simpleStaging" setting on:

  • All files with changes (untracked, unstaged, staged) get listed together in one "changed" list.
  • All files now have a checkmark next to them. This alone determines whether a file ends up in the next commit.
  • On reload, the default state of the checkmarks is unchecked for untracked files, checked for all other files.
  • If no files are checked, commits are disabled.
  • If you switch between repos or branches, your current set of checked/unchecked files is cached and recalled (at least until reload).

@telamonian telamonian marked this pull request as ready for review November 13, 2019 03:14
@telamonian
Copy link
Member Author

Wishlist for future PRs:

  • master checkbox (ie check/uncheck all) to the left of the "Changed" header
  • restore cache of marked files on reload

@kgryte
Copy link
Member

kgryte commented Nov 15, 2019

Tested this locally. As far as I can tell, the proposed functionality of a simplified staging area works.

One thing I think could be improved is the polling duration. Currently, the Git status is refreshed every 10 seconds. I think this should be reduced to as little as 1 second.

Otherwise, as a user, I experience a significant lag between when I save my changes and when I see those changes appear in the staging area, and experience a similar lag when I go to commit my changes and still see the file appearing as "changed" in the staging panel.

There may be some concern regarding having too high a polling rate. On the client side this is less of a concern due to the event loop; on the server, we may need to transition the Tornado HTTP handlers to use coroutines and the Git methods to use asyncio.

Your thoughts @fcollonval (as I believe you last touched the code which sets the default refresh interval)?

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for this really nice new feature. It works as advertised.

I left small comments.
As mentioned in the discussion #448 (comment), I think the pooling interval is too large; I let you decide what would be a better value.

src/model.ts Outdated Show resolved Hide resolved
src/model.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
@telamonian
Copy link
Member Author

Let's not handle the polling thing here, since it's an issue independent of whether simple staging is on or not.

Since the jlab settings editor remains a bit impenetrable, I just now added an item to the "Git" menu that toggle simple staging on/off.

The "Git" menu is in need of a reorg (we definitely don't need a menu item that clicks the sidebar button for the user, and a menu item that opens a new terminal and cd's to the current dir is too general. It should maybe be in the main jlab "File" menu, tho). That's yet another separate issue though.

@fcollonval
Copy link
Member

Since the jlab settings editor remains a bit impenetrable, I just now added an item to the "Git" menu that toggle simple staging on/off.

I'm sorry I did not catch what you mean by that. What is the problem you would like to address?

@fcollonval fcollonval mentioned this pull request Nov 22, 2019
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thx

@fcollonval fcollonval merged commit a78cac8 into jupyterlab:master Nov 22, 2019
telamonian added a commit to telamonian/jupyterlab-git that referenced this pull request Dec 4, 2019
fixes committing more than one file at a time in simple staging mode
telamonian added a commit that referenced this pull request Dec 4, 2019
followup #448: fix git add multiple files
@kgryte kgryte mentioned this pull request Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants