jisaacks / GitGutter

A Sublime Text 2/3 plugin to see git diff in gutter
MIT License
3.88k stars 224 forks source link

Show staging status #118

Open mcmillion opened 11 years ago

mcmillion commented 11 years ago

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

jisaacks commented 11 years ago

This is a feature I am currently working on.

Thanks.

tkoenig commented 10 years ago

+1

jisaacks commented 10 years ago

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

Some notes:


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 commented 10 years ago

Just got this worked into my theme. Looks great.

maffiou commented 10 years ago

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 commented 10 years ago

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

jisaacks commented 10 years ago

@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 commented 10 years ago

@jisaacks Sorry for not responding earlier, I've been stupidly busy lately. I'm using gitGutter every single day and love it. :+1: 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 commented 10 years ago

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

Raynos commented 10 years ago

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

This might be an issue for https://github.com/EleazarCrusader/nexus-theme/issues/9

Raynos commented 10 years ago

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 commented 10 years ago

@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:

Then for this branch, it adds:

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:

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 commented 10 years ago

@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 commented 10 years ago

@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 commented 9 years ago

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

... and the scopes in DeepBlueSee:

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.

Adarma commented 6 years ago

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:

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:

Any plan to set colours a similar way?

deathaxe commented 6 years ago
  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 commented 6 years ago

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.