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

Show staging status #118

Open
mcmillion opened this issue Oct 13, 2013 · 26 comments
Open

Show staging status #118

mcmillion opened this issue Oct 13, 2013 · 26 comments

Comments

@mcmillion
Copy link

Would it be possible to show an indicator for lines of files that have been staged?

@jisaacks
Copy link
Owner

This is a feature I am currently working on.

Thanks.

@tkoenig
Copy link

tkoenig commented Oct 15, 2013

+1

@jisaacks
Copy link
Owner

I have a branch called staged with what I have so far.

Some notes:

  • Trying to determine what changes are staged/unstaged in a dirty file (a file with unsaved changes) is very difficult. so I currently only have it distinguishing unstaged/staged when the file is clean (no unsaved changes.) If anyone can solve this problem it would be great!
  • The colors for the icons come from the colors scheme. GitGutter uses the format markup.change-type.git_gutter for example markup.inserted.git_gutter. Since color schemes in sublime text fall back, if the current color scheme does not define a color for markup.inserted.git_gutter it will fall back and use markup.inserted which many of the default color schemes have defined. I have extended this farther by making the colors for staged and unstaged defined like markup.inserted.git_gutter.staged and markup.inserted.git_gutter.unstaged. So needless to say if you want to checkout the staged branch and check it out, you will need to define these color defs. But it won't break if you don't have them defined. I have already added them to DeepBlueSee so you can use that color scheme to check this feature out.

Here is a screenshot using the DeepBlueSee color scheme:
staged
The green ones are staged and the faint ones are not staged.


I would love to have some feedback on all of this from any interested parties.

@mcmillion
Copy link
Author

Just got this worked into my theme. Looks great.

@maffiou
Copy link

maffiou commented Nov 6, 2013

Cool stuff.

I've just started to use gitGutter and it's amazing...
I do a lot of patch amending, would it be possible to add a key combination to show the diffs between HEAD and HEAD^ as well as the ongoing changes (I appreciate it might be quite tricky)

@jisaacks
Copy link
Owner

jisaacks commented Nov 7, 2013

@maffiou take a look at the compare branch and this issue: #104

@jisaacks
Copy link
Owner

jisaacks commented Feb 2, 2014

@maffiou There is a new feature (pulled in from the branch I linked previously) that lets you choose a commit to compare against. You can find it in the command palette.

@mcmillion I am still working on this, its quite tricky. I have a pull request #136 but I am not convinced it is ready for prime time just yet. It does show the staged status even if the file is dirty now.

If you pull the latest code, I would love any feedback.

@maffiou
Copy link

maffiou commented Feb 6, 2014

@jisaacks Sorry for not responding earlier, I've been stupidly busy lately. I'm using gitGutter every single day and love it. 👍 Will try the latest version when I can risk it. :)
FYI: I did try to flattr you but it doesn't seem to have been picked up...

@jisaacks
Copy link
Owner

jisaacks commented Feb 6, 2014

@maffiou Thanks man! BTW, did not know about flattr, just went and created an account.

@Raynos
Copy link

Raynos commented Apr 30, 2014

@jisaacks I just tried your branch. I'm afraid it won't work cleanly with my default theme :(

This might be an issue for MarkHMorrison/nexus-theme#9

@Raynos
Copy link

Raynos commented Apr 30, 2014

It might also be an issue with https://github.com/Benvie/JavaScriptNext.tmLanguage/blob/master/JavaScriptNext.tmTheme#L1493

not sure how to apply patches or where to apply them.

@jisaacks
Copy link
Owner

@Raynos I am not exactly sure I understand what your issue is, but I think you are saying that of the 2 themes you want to use, Nexus, and JavaScriptNext, neither define the appropriate values needed for this branch.

The values that need to be defined (for git gutter in general) look like this:

  • markup.inserted.git_gutter
  • markup.deleted.git_gutter
  • markup.modified.git_gutter

Then for this branch, it adds:

  • markup.inserted.git_gutter.staged
  • markup.deleted.git_gutter.unstaged
  • markup.modified.git_gutter.staged_unstaged (for lines that are mixed)

The way these color defs work in sublime text, is, each . narrows the scope, but falls back to the previous scope if not defined.

So when something needs to be colored like markup.inserted.git_gutter.staged sublime checks for the following values and uses the first one it finds:

  • markup.inserted.git_gutter.staged
  • markup.inserted.git_gutter
  • markup.inserted
  • markup

It looks like those themes define the markup.inserted etc. without the git_gutter or staged pieces, which should still work, except staged/unstaged changes will not be colored differently.

If you want to modify/patch the theme to make it work as desired for this branch, you just need to edit the theme and add the appropriate values.

Alternatively if you just want to grab a theme to play with this branch that already has the needed color defs, you can use this theme

@Raynos
Copy link

Raynos commented May 1, 2014

@jisaacks can we get these changes merged into master behind opt in config flags.

It makes it easier to try these changes without having to manually install a branch of gitgutter in sublime.

Same thing for the protected regions branch.

@jisaacks
Copy link
Owner

jisaacks commented May 1, 2014

@Raynos actually, I think there is a way to tag a branch and have it installable as a separate version via Package Manager. I will look into that when I get some free cycles. I would rather not merge half baked features into master.

@Adarma
Copy link

Adarma commented Mar 25, 2015

@jisaacks
I want to add the colours to my theme, but notice some inconsistencies between the scopes you mention:

  • scope: markup.inserted.git_gutter
  • scope: markup.deleted.git_gutter
  • scope: markup.modified.git_gutter
  • scope: markup.inserted.git_gutter.staged
  • scope: markup.deleted.git_gutter.unstaged
  • scope: markup.modified.git_gutter.staged_unstaged

... and the scopes in DeepBlueSee:

  • scope: markup.deleted.git_gutter, markup.deleted.git_gutter.staged
  • scope: markup.inserted.git_gutter, markup.inserted.git_gutter.staged
  • scope: markup.changed.git_gutter, markup.changed.git_gutter.staged
  • scope: markup.deleted.git_gutter.unstaged
  • scope: markup.inserted.git_gutter.unstaged
  • scope: markup.changed.git_gutter.unstaged
  • scope: markup.ignored.git_gutter
  • scope: markup.untracked.git_gutter

Some Questions:

Should it be markup.modified... or markup.changed... or do they do different things?
Is markup.modified.git_gutter.staged_unstaged used?
How is markup.ignored.git_gutter used?
how is markup.untracked.git_gutter used?

Could this be merged and released yet?

@RafalSkolasinski
Copy link

RafalSkolasinski commented Feb 6, 2017

Hmm... I am not sure if this should be separate issue or comment here... I was checking possible options for compare_against and I think it's missing comparing against staged area. In the way that showed changes are consistent with git diff output.

@deathaxe
Copy link
Collaborator

I like this idea as it should by quite easy to realize and seems straight forward. I was thinking about the approach suggested by PR #136 to combine diffs from staging area and HEAD but found it quite heavy.

I think I'll pick up this idea.

@deathaxe deathaxe self-assigned this Feb 26, 2017
@deathaxe
Copy link
Collaborator

deathaxe commented Mar 8, 2017

An idea how to handle Compare against staging area

I find selecting the Compare against staging area manually possibly not to be so user friendly and a bit tricky on the other hand.

  1. From user's view I would prefere to automatically compare against the staged file if exists.
  2. From the developer's view it is a bit tricky to set the compare target manually, because I can't compare against an empty staging area. Automatically switching compare target back to HEAD if a view does not show staged file might confuse users. The compare target is set per repository. Not all open views may show a staged file. If user changes views ... what to do?

So here is an idea:

GitGutter compares against the HEAD by default, but can compare the view against the current branch, too.

When comparing against...

  1. HEAD: The view is automatically compared against the staging area if not empty.
  2. current branch: The view is always compared against the latest commit.

These two options would allow comparing against both staging area and latest commit without any additional command.

So all change markers will disapear as soon as a file gets staged. The staging status is shown in the status bar. Any further changes will start to add new gutter icons and the status bar text displays staged and modified.

@jisaacks
Copy link
Owner

jisaacks commented Mar 8, 2017

@deathaxe I am not sure I follow. What is the difference between HEAD and the current branch? Don't the both point to the same commit?

@deathaxe
Copy link
Collaborator

deathaxe commented Mar 9, 2017

Yes this is the point. The question is, how and when to enable people to compare against the staging area.

One option would be to add a Compare against staging area command as suggested by @RafalSkolasinski. The user would need to activly select the staging area to compare against by keybinding, command pallet or setting.

But if a view's file is not in the staging area, GitGutter needs to fallback to the HEAD anyway to have something to compare against. This can happen in two ways:

  1. Keep user selected compare target and simply compare against the HEAD.
  2. Reset user selected compare target from 'staged' to 'HEAD'.

Option 2 interferes with the feature of selecting a compare target per repository. Example: 2 tabs showing files of the same repo are open. 1 staged, 1 modified. User has the staged view on top editing something while compared against staging area. Now he switches to second (modified) tab. The compare target is reset to HEAD as not contained in the staging area. Switching back to the staged view, would now no longer compare against staging area as HEAD is the new target. -> No-Go.

Option 1 is the better idea, as it would automatically compare against HEAD for unstaged files and compare against staging area if a file is staged. The new command would be named like Compare against HEAD or staging area then.

So the question is: Do I need to add this explicit command and setting value for it or can I automatically compare against staging area, if HEAD is the selected compare target?

Doing so would extend the meaning of HEAD to include a staged file if available automatically without user interaction. If a user still wants to compare against the latest committed file the Compare against branch command could be used to select the current branch, which would not include staging area.

@Adarma
Copy link

Adarma commented Mar 9, 2017

I think you are going down the rabbit hole with this comparing to the staging area stuff.

The idea is to 'simply' compare to the HEAD as usual, but have different icons to indicate if the hunk is staged or not.

[I do realize this is not actually simple to implement...]

@deathaxe
Copy link
Collaborator

deathaxe commented Mar 9, 2017

This is what I wanted to avoid because this requires to read both the staged and commited file, compare both to the view and merge results. This costs much more effort and time on large files. Even worse, a new scope is needed for a new set of icons, which will then need to be added to all color schemes to show up correctly. This is the approach from PR #136.

Just switching between staged and commited files would just need to add one further method to read the staged file content. Nothing else need to be changed. The staging are is handled like any other compare target.

@jisaacks
Copy link
Owner

jisaacks commented Mar 9, 2017

The idea is to 'simply' compare to the HEAD as usual, but have different icons to indicate if the hunk is staged or not.

This is precisely what I was attempting to solve in my branch. I was getting results but its a very complex problem. Probably more complex than all of the rest of git gutter. And it's an edge case. Not worth the complexity in my opinion. Although it would be cool.

deathaxe added a commit that referenced this issue Jul 5, 2017
The open view can be compared against its staged counter part, if available as
comparing against any other target is possible by a command.

Known Issues:

As a file from staging area can be read by `git show` only, this won't work
with smudge filters.
deathaxe added a commit that referenced this issue Sep 10, 2017
The open view can be compared against its staged counter part, if available as
comparing against any other target is possible by a command.

Known Issues:

As a file from staging area can be read by `git show` only, this won't work
with smudge filters.
@Adarma
Copy link

Adarma commented Jul 4, 2018

Was there any progress on this? I see there is still a "staged" branch.
Does compare against branch now compare to the staging area if staged?

I'm upgrading a colour scheme to the "sublime-color-scheme" format and have these scopes:

  • markup.deleted.git_gutter, markup.deleted.git_gutter.staged
  • markup.inserted.git_gutter, markup.inserted.git_gutter.staged
  • markup.changed.git_gutter, markup.changed.git_gutter.staged
  • markup.deleted.git_gutter.unstaged
  • markup.inserted.git_gutter.unstaged
  • markup.changed.git_gutter.unstaged
  • markup.ignored.git_gutter
  • markup.untracked.git_gutter

Are all these scopes still used?

The new SublimeLinter4 sets colours via its settings file rather than custom scopes defined in the color scheme. It has this setting to set the error colour:

  • "scope": "region.redish markup.error.sublime_linter"

Any plan to set colours a similar way?

@deathaxe
Copy link
Collaborator

deathaxe commented Jul 4, 2018

  1. The .staged and .unstaged scopes were never used in any GitGutter release.

  2. The staged branch is quite outdated and contains some unsolved issues in ways how to handle regions which were changed in both the staging area and the worktree.

  3. I tried to create a way to support the index as normal compare target such as any other branch/tag/commit in the status_porcelain2 branch, but it broke support for older git versions (<2.10) and introduces a couple of issues and complexities with all the existing "Compare against functions". Therefore I did not progress on this path. The most tricky thing is to track the index for changes to avoid checking out its content after each keypress. I found a possible way, but it would require some bigger code changes I simple did not find the time/power for.

  4. I know about the region.redish and co, but I find them too blurry. The correct scope for such kinds of regions is simply markup.. Therefore it is not planed to support the region. ... stuff.

@r-stein
Copy link
Collaborator

r-stein commented Jul 4, 2018

If you use the scope region.redish markup.deleted.git_gutter it will only use the color of region.redish if the colorscheme does not define a color for markup or markup.deleted or markup.deleted.git_gutter, imo it combines the best of both world. You always have color and it is not everything white, but a colorscheme can overwrite that color.

It looks like it would need a refactoring, but except from that I don't really see disadvantages. Nonetheless I am not sure whether we should expose that to the settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants