FredrikNoren / ungit

The easiest way to use git. On any platform. Anywhere.
MIT License
10.44k stars 637 forks source link

Patch functionality doesn't add/remove lines to index #730

Open dbkaplun opened 8 years ago

dbkaplun commented 8 years ago

When you click Patch, you are able to select lines you want to commit or not commit. However, when you look at git status, it still shows files as unstaged, even though the lines are selected. ungit should store the lines to add/remove as staged/unstaged changes.

jung-kim commented 8 years ago

This was intended and this can mess with git commit as it is breaking the assumption.

@FredrikNoren how do you feel about this?

FredrikNoren commented 8 years ago

Well the model right now is that ungit doesn't change the git state in staging until you hit commit. The reason for that originally was mainly because it's a lot simpler from a UI perspective. Git can be in a lot of weird states (for instance a file can be both staged and have changes to it that aren't staged yet) which makes the UI a lot more complicated. For instance to commit new changes right now you only have to type in the commit message and hit commit, but if it was following git more closely you'd probably have to stage files first ("add all").

It's probably totally doable to find a UI that works like that, but it would also be a challenge to make that intuitive and keep the feeling of "easy" that the rest of ungit tries to give.

dbkaplun commented 8 years ago

@FredrikNoren from my understanding, the current behavior is that ungit has its own "staging area" stored in memory that is lost as soon as the browser window is closed. If that's true, then there should be no change in UI at all. The only difference would be that the actual git index would store staged lines, rather than memory. IMHO, it would be more user-friendly as it more closely matches git itself (as well as the behavior of other git GUIs).

waldyrious commented 8 years ago

I was also confused by this (granted, I'm a frequent user of git add -p). I think at the very least the different behavior of ungit (from what I believe to be the reasonable expectation from someone learning about git) should be explicit, e.g. by an informative tooltip in the patch button. But ideally I'd favor @dbkaplun's proposal of using git's native staging area.

jung-kim commented 8 years ago

I do see the value of this ideology. Have ungit use git's staging properly thus have it tightly coupled to git. However, I question the value of implementing this ideology to as technical cost of implementation of this can be monumental. For example, this means that we need to do git add -p call for every single mouse click on git patch line. This is possible, but it seems error prone and many unnecessary traffic. Not to mention all of ungit's api calls needs to handle various states git can be in as Fredrick mentioned, which is not an easy task.

For me, I'd prefer to keep it as it is and have better documentation to note this behavior. I will try to think of an way to more clearly state this in documentation but if you guys would like to do that please feel free to submit PRs.