[Feature request] Add command to stage/unstage hunk/line at the cursor

[cecep2]

Thank you for this great work! I know that I can stage hunks or lines using the inline diff view and I use this feature frequently. In addition to that, I think it would be nice if I could directly stage/unstage the hunk or line at the cursor by calling a command like git: stage hunk or git: stage line from the command palette or via a keyboard shortcut in the normal editor window (similar to vim-gitgutter). This way, there would be no need to open the inline diff view first if you just quickly want to stage minor changes like typo corrections. Sorry if this has been asked before.

[kaste]

Of course this would be excellent, and let's just assume the commands wouldn't be that hard to write. But how would the UI look like? The user needs some indication of where the hunk starts and ends. Red/deleted lines are also never visible in a standard view. So this rules out line-by-line staging already, I think.

How do you imagine this to work?

[cecep2]

Doesn't Sublime Text 3 already ship with a user indication for hunks: diff markers in the gutter? It would then work exactly as in vim-gitgutter: You select the hunk/line indicated by the diff markers and call a command like git: stage hunk. This works for both additions-only hunks and deleted hunks.

[kaste]

Hm. For deleted hunks, you literally cannot be on such a line, you can be on the line before or after though. This could work for the git_diff_target=index mode because then the markers actually disappear after staging such a hunk. In "head" mode they wouldn't.

[cecep2]

I don't know how vim-gitgutter does it, but there you can select the line where the gutter indicates a deletion and stage the deletion. My guess is that while you don't literally select the deleted line in vim-gitgutter, it uses the position of the cursor somehow?

[kaste]

Another problem of ordinary views is that they can be unsaved. You get the Sublime markers regardless of the save state, although they often change when saving, as if Sublime recomputes the diff, but staging only works after saving. It would get confusing rather quickly otherwise. Imagine you would commit without actually ever saving.

[cecep2]

I'm not very familiar with Sublime's API, but could GitSavvy check if the file contains unsaved changes and then just print a helpful error message like "Please save your latest changes before staging/unstaging"?

[kaste]

Yeah, sure that would work. Now, that sounds interesting and doable. I don't even have the Sublime's git feature turned on, and usually prefer the "HEAD" mode, because it adds so much UI interfaces I don't need. Esp. the direct marketing of Sublime Merge in the context menu is so annoying. I first would need to get rid of this, and ideally also the branch button in the status bar because I prefer the GitSavvy information here (I just need dirty yes/no, branch name and ahead/behind signs).

[cecep2]

I agree that the marketing for Sublime Merge is annoying. I still use the build-in git features because I simply prefer using build-in functionality over extensions if I can. If you just want diff markers with no marketing, I think https://github.com/jisaacks/GitGutter looks good :-)

Thank you again for the great work and for being so responsive, really hope to see this feature :-)

[kaste]

An implementation is available at #1305. Please test this thoroughly. As there is some math involved esp. look for edge cases where we could have +/-1 errors. Look that after applying a patch the patch is actually applied at the correct position. Not only the Sublime markers should go away but the raw diff (git diff (cached)) must look sane. (We compute the patches here manually so we probably never get an error from git here that a patch does not apply. T.i. we don't have the safety net git usually gives us!)

Single cursors, multiple cursors, as well as single or multiple selections should be supported. Test especially multiple cursors/selections over a whole file with gaps in it. (If we have 4 changes, e.g. only stage 2 and 4 because that is the tricky part of the patch generation.)

(For testing PR's: Open the Packages folder, git clone https://github.com/timbrel/GitSavvy.git into it, then checkout the branch. Restart Sublime.)

[cecep2]

Thank you! I've done some quick testing (using the keyboard shortcut you suggested) and staged some single additions-only hunks, multiple additions-only hunks selected with multiple cursors and deletion-only hunks. I checked git diff cached each time and everything worked as expected. I will keep using this in the next days and let you know if I encounter any issues.

I love that the diff markers immediately disappear, and I can use Ctrl+. and Ctrl+, to jump between modifications and quickly stage minor changes now :-) I only directly realized that an 'undo last staging' command would be useful too, but this already looks like a great first step.

[kaste]

I love that the diff markers immediately disappear, and I can use Ctrl+. and Ctrl+,

Yeah, the UX is really great.

I also thought about the "undo". But maybe only because this would speed up testing. I actually don't use the undo feature often when using the specialized views (diff/inline diff) so maybe that need goes away in real programming life. It is somewhat not realistic that users bind yet another command to some key-combo, only commands you use 100 times a day should get a key-combo otherwise you forget about it anyway, and then it is not clear to me how this "undo" feature should be accessed.

I also wonder if there is a need for the "reset hunk" feature which is common in the other views.

[cecep2]

True, I also rarely use 'undo staging' in general. It's certainly not high priority to implement and I wouldn't make a keybinding for it either, but being able to call it in the command palette would be nice in those occasional instances where want to undo the staging I just did. Being able to stage by line in the normal editor window would be higher priority for me - but staging hunks is what I do most often by far, which the current implementation already covers :-)

[kaste]

The question is how stage-by-line could work here.

For example, for this view

I have three "modified" lines I could then stage. But the underlying diff only has two removed lines

In that scenario it is impossible to produce valid line-patches.

[cecep2]

Line staging would only be available for hunks without deletions (additions-only or modifications), but that would still useful I think. Not sure if it would be possible or useful to try and utilize Sublime's build-in "Show diff hunk" option (available when you right-click a modified line) to enable line staging in hunks that have deletions.

[asfaltboy]

I love the idea, I tried out the inline staging PR and it works great!

But, after staging, I don't see the diff change in neither mini_diff nor gitgutter. Is this something that should be visualized differently in either (e.g blue color for staged vs green unstaged)?

Note: It would also be nice to have inline unstaging before releasing the feature.

[cecep2]

The diff marker should simply disappear after staging. Are you using the build-in diff markers or an extension like https://github.com/jisaacks/GitGutter?

[asfaltboy]

I tried both ... both stay "green" for me after staging (I can see the stage worked in the status interface)... hmmm, if it works out of the box for you both, then I must have some conflicting/broken settings or packages I guess

[kaste]

@asfaltboy Make sure you have git_diff_target=index set. (This is the Sublime default.) GitGutter probably has the same mode. Show markers against "HEAD" or against the "index".

[kaste]

This implicates a workflow change. If you're used to work against HEAD, you typically make a change, then commit if it works. Then you start rewriting/refactoring the introduced changes, and either amend the previous commit or "revert"/reset if you go in the wrong direction.

A different workflow is to use the "index" area. Here you make a change, then stage/cache the change as soon as if it works. Then you rewrite/refactor and stage again if it still works. Only then you commit.

The introduced shortcut here has some workflow implications. Before that change, I used to work against "HEAD". I edit a view, then switch to the git: diff via key-combo to review, and either hunk specific changes and then c, or I just use C to take everything and go to the commit message view. (For bigger changes I may go to the status view, also via key-combo to review.)

With this new shortcut, I should work against index otherwise the UI doesn't work. So I edit a file, stage directly from the view what I like. Then switch to git: diff (staged)(!) via key-combo to review, maybe unstage a hunk, and then c to write the commit message. (Since I already staged changes in the view it might be even better to jump to the commit message view via key-combo directly, where I can review the stage as well. Unstaging within that commit view should be allowed.)

[cecep2]

Just as a quick follow-up report: I've used this feature regularly since it was introduced and didn't encounter any issues thus far. I'm not sure if any of my uses represented 'edge cases', but it seems to work well. I would still suggest an 'unstage hunk' and an 'undo staging' command. Line staging would be nice too but perhaps more complicated.

[kaste]

Yeah, the command works reliable but the diff markers coming from Sublime are "buggy".

E.g. here I added L106-123, but it looks like I could stage on L124

But the inline diff shows

I get the same for removal only lines sometimes. Just if in doubt, git produces usually the better diff.

[kaste]

Another example: here I removed some lines starting from line 215 where the cursor. Before saving, there is a removal indicator between L214 and L215:

After saving the marker jumps to L218/19

To hunk this, L214 is correct, but the UI/UX is completely lacking.

[kaste]

You can try #1306 (which is on top of the feature here) which greatly improves the inline diff workflow. See the description over there.

[cecep2]

Thank you! I really like this and I agree that it perfectly complements the feature discussed here. For me this is much more intuitive than the old version! I have not much to add other than to say 'thank you, can't wait to see this merged'....but I was actually thinking to suggest something that would nicely complement this new inline diff workflow, so I just throw this in here as food for thought for future versions maybe :-)

In vim-fugitive or vimagit, you can expand or collapse diff views directly within the git status window, and stage/unstage hunks/lines from there as well. Here is a video from vimagit illustrating the idea (starts at about 15 sec): https://asciinema.org/a/28761

This is great for checking many smaller hunk changes across different files without having to switch context often.

In both vimagit and vim-fugitive, expanding a diff view from within the git status window doesn't show you the entire file, only edited hunks plus a few extra lines for context. I think that #1306 would be great in combination with this: if I need more context, I press a keyboard shortcut to see the entire file in the inline diff view, jumping directly to the hunk I just selected in the git status window. Then I do what I must from there and press the same shortcut again to jump back to the git status window.

[kaste]

Yes sure, the interaction between all the views is a main focus. We can now go from the file to the "inline" diff. Let's see if we can go from the "normal" diff to "inline" and vice versa. And of course from the file to the "normal" diff. For the "normal" diff maybe also add collapsing the hunks. (This is what you see in the vim examples.)

I currently have another problem with the different key bindings we have in the different views.

T.i. in the status view, we have [sud] (mnemonic) for staging/unstaging/discarding an item. And [aAUD] staging/unstaging/discarding all. In the diff views, we use [h] for staging/unstaging, but then [H] for discarding. In the "normal" diff view, we also bind ctrl+enter for staging/unstaging and ctrl+backspace for discarding. But not in the "inline" diff view. Lacking are the commands to stage/unstage/discard the whole file.

Usually I prefer the mnemonic bindings, this would lead to [sudSUD]. If we go with ctrl+enter/ctrl+backspace, we had shift (e.g. ctrl+shift+enter) to say: do this for the whole file.

What I don't get though is why ctrl+enter also unstages. This is backwards for me. enter is a positive action, say "commit" or "apply" this hunk, backspace (which usually has a left arrow ◀️ on the keyboard) usually means "no", "revert" ... so it is a natural fit for "ah no, do not commit this yet" (unstage) and "just revert this change altogether" (discard).

But maybe that's just me.

EDIT: I think before we had the [TAB] switching just h was maybe enough, simply because it was an effort to switch the modes. But now, you can just flip-flip between them, very quickly, and get lost. If we had the explicit [sud] the user always presses the right key, expressing the intention, and we can just flash the status bar in the wrong mode, e.g. "You already staged this hunk!", "You can't discard a staged hunk!", "Hunk is already not staged.".

[kaste]

@asfaltboy @stoivo @divmain Are you still here. Have an opinion or history about the key bindings we have here? Above comment https://github.com/timbrel/GitSavvy/issues/1304#issuecomment-632901535

[asfaltboy]

I personally never used enter ctrl+enter to stage/unstage. I just use the mnemonics you mentioned in status dash and h/l in diffs. That said, we can change the defaults if you rather, I don't have a strong opinion on it. Flashing indicators, when an action shouldn't be done, makes sense to me.

[kaste]

I also just need more bindings like the uppercase [SUD] to apply the action on the whole file. With the current [hH] I would need to make them [ctrl+h] and [ctrl+shift+h] but we don't use ctrl as a modifier elsewhere.

[asfaltboy]

I think ctrl and ctrl+shift are reasonable, though myself, I probably wouldn't find myself staging a whole file from the diff view too often.

[kaste]

I'm inclined to release these two PR's. So please raise your hand if there is something completely stupid here (besides some trivial rebasing action this will need).

Should I rename the new command introduced here from gs_inline_stage_hunk to just gs_stage_hunk? (Or something different?) Otherwise I remove the one print statement, rebase to match the current flake8 linter version, and just go on.

[kaste]

Note the command has been renamed to gs_stage_hunk.