git-gui: accommodate for intent-to-add files

From: Johannes Schindelin <[email protected]>

As of Git v2.28.0, the diff for files staged via git add -N marks them as new files. Git GUI was ill-prepared for that, and this patch teaches Git GUI about them.

Please note that this will not even fix things with v2.28.0, as the rp/apply-cached-with-i-t-a patches are required on Git's side, too.

This fixes https://github.com/git-for-windows/git/issues/2779

Signed-off-by Johannes Schindelin <[email protected]>

---
git-gui: accommodate for intent-to-add files

This fixes the intent-to-add bug reported in
https://github.com/git-for-windows/git/issues/2779: after a file was staged with git add -N, staging hunks/lines would fail silently.

On its own, this patch is not enough, as it requires the patches provided in rp/apply-cached-with-i-t-a to be applied on Git's side.

Please note that this patch might need a bit more help, as I do not really know whether showing "new file mode 100644" in the diff view is desirable, or whether we should somehow try to retain the "intent-to-add" state so that unstaging all hunks would return the file to "intent-to-add" state.

Thoughts?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-699%2Fdscho%2Fgit-gui-stage-ita-hunks-and-lines-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-699/dscho/git-gui-stage-ita-hunks-and-lines-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/699

 git-gui.sh   |  2 ++
 lib/diff.tcl | 12 ++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

git-gui.sh +2 -0

index 49bd86e635..e08cb17395 100755
@@ -2080,6 +2080,7 @@ set all_icons(U$ui_index)   file_merge
 set all_icons(T$ui_index)   file_statechange
 
 set all_icons(_$ui_workdir) file_plain
+set all_icons(A$ui_workdir) file_plain
 set all_icons(M$ui_workdir) file_mod
 set all_icons(D$ui_workdir) file_question
 set all_icons(U$ui_workdir) file_merge

@@ -2106,6 +2107,7 @@ foreach i {
 		{A_ {mc "Staged for commit"}}
 		{AM {mc "Portions staged for commit"}}
 		{AD {mc "Staged for commit, missing"}}
+		{AA {mc "Intended to be added"}}
 
 		{_D {mc "Missing"}}
 		{D_ {mc "Staged for removal"}}

lib/diff.tcl +6 -3

index 871ad488c2..36d3715f7b 100644
@@ -582,7 +582,8 @@ proc apply_or_revert_hunk {x y revert} {
 	if {$current_diff_side eq $ui_index} {
 		set failed_msg [mc "Failed to unstage selected hunk."]
 		lappend apply_cmd --reverse --cached
-		if {[string index $mi 0] ne {M}} {
+		set file_state [string index $mi 0]
+		if {$file_state ne {M} && $file_state ne {A}} {
 			unlock_index
 			return
 		}

@@ -595,7 +596,8 @@ proc apply_or_revert_hunk {x y revert} {
 			lappend apply_cmd --cached
 		}
 
-		if {[string index $mi 1] ne {M}} {
+		set file_state [string index $mi 1]
+		if {$file_state ne {M} && $file_state ne {A}} {
 			unlock_index
 			return
 		}

@@ -687,7 +689,8 @@ proc apply_or_revert_range_or_line {x y revert} {
 		set failed_msg [mc "Failed to unstage selected line."]
 		set to_context {+}
 		lappend apply_cmd --reverse --cached
-		if {[string index $mi 0] ne {M}} {
+		set file_state [string index $mi 0]
+		if {$file_state ne {M} && $file_state ne {A}} {
 			unlock_index
 			return
 		}

base-commit: 469725c1a3d44f7e1475f1d37cd13e0824d4ea41

Signature · 1 line

gitgitgadget

Quoted 19 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

As of Git v2.28.0, the diff for files staged via git add -N marks them as new files. Git GUI was ill-prepared for that, and this patch teaches Git GUI about them.

Please note that this will not even fix things with v2.28.0, as the rp/apply-cached-with-i-t-a patches are required on Git's side, too.

This fixes https://github.com/git-for-windows/git/issues/2779

Signed-off-by Johannes Schindelin <[email protected]>

---
git-gui: accommodate for intent-to-add files

This fixes the intent-to-add bug reported in
https://github.com/git-for-windows/git/issues/2779: after a file was staged with git add -N, staging hunks/lines would fail silently.

Quoted 8 lines

On its own, this patch is not enough, as it requires the patches provided in rp/apply-cached-with-i-t-a to be applied on Git's side.

Please note that this patch might need a bit more help, as I do not really know whether showing "new file mode 100644" in the diff view is desirable, or whether we should somehow try to retain the "intent-to-add" state so that unstaging all hunks would return the file to "intent-to-add" state.

I built latest Git master (e9b77c84a0) which has rp/apply-cached-with-i-t-a and tested this patch. It works... for the most part.

I can select a line set of lines and they get staged/unstaged, which is good. The part that is not good though is that a lot of common operations still don't work as they should:

- I can't click on the icon in the "Unstaged Changes" pane to stage the whole file. Nothing happens when I do that.

- The file that is marked as intent-to-add shows up in both the "Staged Changes" and "Unstaged Changes" panes, with the "Staged Changes" part being empty. Ideally it should only show up in the "Unstaged Changes" pane.

- Selecting the whole file and choosing "Stage Lines for Commit" works well. But choosing "Stage Hunk for Commit" does not. While the changes do get staged, the UI is not properly updated and the file is still listed in the "Unstaged Changes" pane.

I think the difference here is because for apply_or_revert_range_or_line, we call do_rescan after it to update the UI, but for apply_or_revert_hunk we update the UI "manually" in the function after we are done applying or reverting the changes. So the logic to update the UI needs to be updated to account for this change. Or we can get rid of all that logic and just run a rescan.

And also, like you mentioned, we don't retain the i-t-a state when unstaging. But with some quick testing, I see that Git command line doesn't either (I tried a plain git restore --staged). So IMO we should mimic what the command line UI does and not retain the i-t-a state when unstaging.

Hi Pratyush,

Quoted 58 lines · Pratyush Yadav

Quoted 19 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

As of Git v2.28.0, the diff for files staged via git add -N marks them as new files. Git GUI was ill-prepared for that, and this patch teaches Git GUI about them.

Please note that this will not even fix things with v2.28.0, as the rp/apply-cached-with-i-t-a patches are required on Git's side, too.

This fixes https://github.com/git-for-windows/git/issues/2779

Signed-off-by Johannes Schindelin <[email protected]>

---
git-gui: accommodate for intent-to-add files

This fixes the intent-to-add bug reported in
https://github.com/git-for-windows/git/issues/2779: after a file was staged with git add -N, staging hunks/lines would fail silently.

Quoted 8 lines

On its own, this patch is not enough, as it requires the patches provided in rp/apply-cached-with-i-t-a to be applied on Git's side.

Please note that this patch might need a bit more help, as I do not really know whether showing "new file mode 100644" in the diff view is desirable, or whether we should somehow try to retain the "intent-to-add" state so that unstaging all hunks would return the file to "intent-to-add" state.

I built latest Git master (e9b77c84a0) which has rp/apply-cached-with-i-t-a and tested this patch. It works... for the most part.

I can select a line set of lines and they get staged/unstaged, which is good. The part that is not good though is that a lot of common operations still don't work as they should:

- I can't click on the icon in the "Unstaged Changes" pane to stage the whole file. Nothing happens when I do that.

- The file that is marked as intent-to-add shows up in both the "Staged Changes" and "Unstaged Changes" panes, with the "Staged Changes" part being empty. Ideally it should only show up in the "Unstaged Changes" pane.

- Selecting the whole file and choosing "Stage Lines for Commit" works well. But choosing "Stage Hunk for Commit" does not. While the changes do get staged, the UI is not properly updated and the file is still listed in the "Unstaged Changes" pane.

I think the difference here is because for apply_or_revert_range_or_line, we call do_rescan after it to update the UI, but for apply_or_revert_hunk we update the UI "manually" in the function after we are done applying or reverting the changes. So the logic to update the UI needs to be updated to account for this change. Or we can get rid of all that logic and just run a rescan.

Quoted 5 lines

And also, like you mentioned, we don't retain the i-t-a state when unstaging. But with some quick testing, I see that Git command line doesn't either (I tried a plain git restore --staged). So IMO we should mimic what the command line UI does and not retain the i-t-a state when unstaging.

To be quite honest, I had hoped that this might be a good patch to start from... for somebody else (you?)

Quoted 66 lines · Johannes Schindelin

Quoted 63 lines · Pratyush Yadav

Quoted 19 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

As of Git v2.28.0, the diff for files staged via git add -N marks them as new files. Git GUI was ill-prepared for that, and this patch teaches Git GUI about them.

Please note that this will not even fix things with v2.28.0, as the rp/apply-cached-with-i-t-a patches are required on Git's side, too.

This fixes https://github.com/git-for-windows/git/issues/2779

Signed-off-by Johannes Schindelin <[email protected]>

---
git-gui: accommodate for intent-to-add files

This fixes the intent-to-add bug reported in
https://github.com/git-for-windows/git/issues/2779: after a file was staged with git add -N, staging hunks/lines would fail silently.

Quoted 8 lines

On its own, this patch is not enough, as it requires the patches provided in rp/apply-cached-with-i-t-a to be applied on Git's side.

Please note that this patch might need a bit more help, as I do not really know whether showing "new file mode 100644" in the diff view is desirable, or whether we should somehow try to retain the "intent-to-add" state so that unstaging all hunks would return the file to "intent-to-add" state.

I built latest Git master (e9b77c84a0) which has rp/apply-cached-with-i-t-a and tested this patch. It works... for the most part.

I can select a line set of lines and they get staged/unstaged, which is good. The part that is not good though is that a lot of common operations still don't work as they should:

- I can't click on the icon in the "Unstaged Changes" pane to stage the whole file. Nothing happens when I do that.

- The file that is marked as intent-to-add shows up in both the "Staged Changes" and "Unstaged Changes" panes, with the "Staged Changes" part being empty. Ideally it should only show up in the "Unstaged Changes" pane.

- Selecting the whole file and choosing "Stage Lines for Commit" works well. But choosing "Stage Hunk for Commit" does not. While the changes do get staged, the UI is not properly updated and the file is still listed in the "Unstaged Changes" pane.

I think the difference here is because for apply_or_revert_range_or_line, we call do_rescan after it to update the UI, but for apply_or_revert_hunk we update the UI "manually" in the function after we are done applying or reverting the changes. So the logic to update the UI needs to be updated to account for this change. Or we can get rid of all that logic and just run a rescan.

And also, like you mentioned, we don't retain the i-t-a state when unstaging. But with some quick testing, I see that Git command line doesn't either (I tried a plain git restore --staged). So IMO we should mimic what the command line UI does and not retain the i-t-a state when unstaging.

Quoted 2 lines

To be quite honest, I had hoped that this might be a good patch to start from... for somebody else (you?)

I'll take a stab at this during the weekend :-)

Hi Pratyush,

Quoted 68 lines · Pratyush Yadav

Quoted 66 lines · Johannes Schindelin

Quoted 63 lines · Pratyush Yadav

Quoted 19 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

As of Git v2.28.0, the diff for files staged via git add -N marks them as new files. Git GUI was ill-prepared for that, and this patch teaches Git GUI about them.

Please note that this will not even fix things with v2.28.0, as the rp/apply-cached-with-i-t-a patches are required on Git's side, too.

This fixes https://github.com/git-for-windows/git/issues/2779

Signed-off-by Johannes Schindelin <[email protected]>

---
git-gui: accommodate for intent-to-add files

This fixes the intent-to-add bug reported in
https://github.com/git-for-windows/git/issues/2779: after a file was staged with git add -N, staging hunks/lines would fail silently.

Quoted 8 lines

On its own, this patch is not enough, as it requires the patches provided in rp/apply-cached-with-i-t-a to be applied on Git's side.

Please note that this patch might need a bit more help, as I do not really know whether showing "new file mode 100644" in the diff view is desirable, or whether we should somehow try to retain the "intent-to-add" state so that unstaging all hunks would return the file to "intent-to-add" state.

I built latest Git master (e9b77c84a0) which has rp/apply-cached-with-i-t-a and tested this patch. It works... for the most part.

I can select a line set of lines and they get staged/unstaged, which is good. The part that is not good though is that a lot of common operations still don't work as they should:

- I can't click on the icon in the "Unstaged Changes" pane to stage the whole file. Nothing happens when I do that.

- The file that is marked as intent-to-add shows up in both the "Staged Changes" and "Unstaged Changes" panes, with the "Staged Changes" part being empty. Ideally it should only show up in the "Unstaged Changes" pane.

- Selecting the whole file and choosing "Stage Lines for Commit" works well. But choosing "Stage Hunk for Commit" does not. While the changes do get staged, the UI is not properly updated and the file is still listed in the "Unstaged Changes" pane.

I think the difference here is because for apply_or_revert_range_or_line, we call do_rescan after it to update the UI, but for apply_or_revert_hunk we update the UI "manually" in the function after we are done applying or reverting the changes. So the logic to update the UI needs to be updated to account for this change. Or we can get rid of all that logic and just run a rescan.

And also, like you mentioned, we don't retain the i-t-a state when unstaging. But with some quick testing, I see that Git command line doesn't either (I tried a plain git restore --staged). So IMO we should mimic what the command line UI does and not retain the i-t-a state when unstaging.

Quoted 4 lines

Quoted 2 lines

To be quite honest, I had hoped that this might be a good patch to start from... for somebody else (you?)

I'll take a stab at this during the weekend :-)

Just a gentle ping: did you get a chance to get this patch into a better shape?

Quoted 74 lines · Johannes Schindelin

Quoted 71 lines · Pratyush Yadav

Quoted 68 lines · Johannes Schindelin

Quoted 58 lines · Pratyush Yadav

Quoted 19 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

As of Git v2.28.0, the diff for files staged via git add -N marks them as new files. Git GUI was ill-prepared for that, and this patch teaches Git GUI about them.

Please note that this will not even fix things with v2.28.0, as the rp/apply-cached-with-i-t-a patches are required on Git's side, too.

This fixes https://github.com/git-for-windows/git/issues/2779

Signed-off-by Johannes Schindelin <[email protected]>

---
git-gui: accommodate for intent-to-add files

This fixes the intent-to-add bug reported in
https://github.com/git-for-windows/git/issues/2779: after a file was staged with git add -N, staging hunks/lines would fail silently.

Quoted 8 lines

On its own, this patch is not enough, as it requires the patches provided in rp/apply-cached-with-i-t-a to be applied on Git's side.

Please note that this patch might need a bit more help, as I do not really know whether showing "new file mode 100644" in the diff view is desirable, or whether we should somehow try to retain the "intent-to-add" state so that unstaging all hunks would return the file to "intent-to-add" state.

I built latest Git master (e9b77c84a0) which has rp/apply-cached-with-i-t-a and tested this patch. It works... for the most part.

I can select a line set of lines and they get staged/unstaged, which is good. The part that is not good though is that a lot of common operations still don't work as they should:

- I can't click on the icon in the "Unstaged Changes" pane to stage the whole file. Nothing happens when I do that.

- The file that is marked as intent-to-add shows up in both the "Staged Changes" and "Unstaged Changes" panes, with the "Staged Changes" part being empty. Ideally it should only show up in the "Unstaged Changes" pane.

- Selecting the whole file and choosing "Stage Lines for Commit" works well. But choosing "Stage Hunk for Commit" does not. While the changes do get staged, the UI is not properly updated and the file is still listed in the "Unstaged Changes" pane.

I think the difference here is because for apply_or_revert_range_or_line, we call do_rescan after it to update the UI, but for apply_or_revert_hunk we update the UI "manually" in the function after we are done applying or reverting the changes. So the logic to update the UI needs to be updated to account for this change. Or we can get rid of all that logic and just run a rescan.

Quoted 5 lines

And also, like you mentioned, we don't retain the i-t-a state when unstaging. But with some quick testing, I see that Git command line doesn't either (I tried a plain git restore --staged). So IMO we should mimic what the command line UI does and not retain the i-t-a state when unstaging.

To be quite honest, I had hoped that this might be a good patch to start from... for somebody else (you?)

Quoted 4 lines

Quoted 1 line

I'll take a stab at this during the weekend :-)

Just a gentle ping: did you get a chance to get this patch into a better shape?

No, I have not. I'll try my hand on it tomorrow and try to get it done in time for 2.29.