* [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines @ 2019-08-19 21:41 Pratyush Yadav 2019-08-19 21:41 ` [PATCH 1/3] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav ` (6 more replies) 0 siblings, 7 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-19 21:41 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano Hi, This series adds the ability to revert selected lines and hunks in git-gui. Partially based on the patch by Bert Wesarg [0]. The commits can be found in the topic branch 'py/git-gui-revert-lines' at https://github.com/prati0100/git/tree/py/git-gui-revert-lines Once reviewed, pull the commits from 832064f93d3ad432616e96ca70f682a7cfdcc3e0 till 72eed27a29f1e71cbefefa6b19f96c89793ac74d. Let me know if there is some other way I'm supposed to ask for a pull. [0] https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wesarg@googlemail.com/ Pratyush Yadav (3): git-gui: Move revert confirmation dialog creation to separate function git-gui: Add the ability to revert selected lines git-gui: Add the ability to revert selected hunk git-gui/git-gui.sh | 39 ++++++++++++++++++++++++-- git-gui/lib/diff.tcl | 65 ++++++++++++++++++++++++++++++++++++------- git-gui/lib/index.tcl | 27 +++++++++++------- 3 files changed, 109 insertions(+), 22 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 1/3] git-gui: Move revert confirmation dialog creation to separate function 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav @ 2019-08-19 21:41 ` Pratyush Yadav 2019-08-19 21:41 ` [PATCH 2/3] git-gui: Add the ability to revert selected lines Pratyush Yadav ` (5 subsequent siblings) 6 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-19 21:41 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano Upcoming commits will introduce reverting lines and hunks. They also need to prompt the user for confirmation. Put the dialog creation in its separate function so the same code won't be repeated again and again. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui/lib/index.tcl | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl index b588db11d9..cb7f74af45 100644 --- a/git-gui/lib/index.tcl +++ b/git-gui/lib/index.tcl @@ -388,6 +388,18 @@ proc do_add_all {} { add_helper [mc "Adding all changed files"] $paths } +proc revert_dialog {query} { + return [tk_dialog \ + .confirm_revert \ + "[appname] ([reponame])" \ + "$query" \ + question \ + 1 \ + [mc "Do Nothing"] \ + [mc "Revert Changes"] \ + ] +} + proc revert_helper {txt paths} { global file_states current_diff_path @@ -430,17 +442,12 @@ proc revert_helper {txt paths} { set query [mc "Revert changes in these %i files?" $n] } - set reply [tk_dialog \ - .confirm_revert \ - "[appname] ([reponame])" \ - "$query + set query "$query + +[mc "Any unstaged changes will be permanently lost by the revert."]" + + set reply [revert_dialog $query] -[mc "Any unstaged changes will be permanently lost by the revert."]" \ - question \ - 1 \ - [mc "Do Nothing"] \ - [mc "Revert Changes"] \ - ] if {$reply == 1} { checkout_index \ $txt \ -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav 2019-08-19 21:41 ` [PATCH 1/3] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav @ 2019-08-19 21:41 ` Pratyush Yadav 2019-08-20 19:21 ` Johannes Sixt 2019-08-19 21:41 ` [PATCH 3/3] git-gui: Add the ability to revert selected hunk Pratyush Yadav ` (4 subsequent siblings) 6 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-19 21:41 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano Just like the user can select lines to stage or unstage, add the ability to revert selected lines. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui/git-gui.sh | 25 ++++++++++++++++++++++++- git-gui/lib/diff.tcl | 31 ++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh index 6de74ce639..2011894bef 100755 --- a/git-gui/git-gui.sh +++ b/git-gui/git-gui.sh @@ -3611,9 +3611,15 @@ set ui_diff_applyhunk [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] $ctxm add command \ -label [mc "Apply/Reverse Line"] \ - -command {apply_range_or_line $cursorX $cursorY; do_rescan} + -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} set ui_diff_applyline [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state] +$ctxm add command \ + -label [mc "Revert Line"] \ + -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} +set ui_diff_revertline [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] +set ui_diff_revertline [$ctxm index last] $ctxm add separator $ctxm add command \ -label [mc "Show Less Context"] \ @@ -3711,15 +3717,19 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { set l [mc "Unstage Hunk From Commit"] if {$has_range} { set t [mc "Unstage Lines From Commit"] + set r [mc "Revert Lines"] } else { set t [mc "Unstage Line From Commit"] + set r [mc "Revert Line"] } } else { set l [mc "Stage Hunk For Commit"] if {$has_range} { set t [mc "Stage Lines For Commit"] + set r [mc "Revert Lines"] } else { set t [mc "Stage Line For Commit"] + set r [mc "Revert Line"] } } if {$::is_3way_diff @@ -3730,11 +3740,24 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { || [string match {T?} $state] || [has_textconv $current_diff_path]} { set s disabled + set revert_state disabled } else { set s normal + + # Only allow reverting changes in the working tree. If + # the user wants to revert changes in the index, they + # need to unstage those first. + if {$::ui_workdir eq $::current_diff_side} { + set revert_state normal + } else { + set revert_state disabled + } } + $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l $ctxm entryconf $::ui_diff_applyline -state $s -label $t + $ctxm entryconf $::ui_diff_revertline -state $revert_state \ + -label $r tk_popup $ctxm $X $Y } } diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl index 68c4a6c736..4b2b00df4b 100644 --- a/git-gui/lib/diff.tcl +++ b/git-gui/lib/diff.tcl @@ -640,7 +640,7 @@ proc apply_hunk {x y} { } } -proc apply_range_or_line {x y} { +proc apply_or_revert_range_or_line {x y revert} { global current_diff_path current_diff_header current_diff_side global ui_diff ui_index file_states @@ -660,25 +660,46 @@ proc apply_range_or_line {x y} { if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return - set apply_cmd {apply --cached --whitespace=nowarn} + set apply_cmd {apply --whitespace=nowarn} set mi [lindex $file_states($current_diff_path) 0] if {$current_diff_side eq $ui_index} { set failed_msg [mc "Failed to unstage selected line."] set to_context {+} - lappend apply_cmd --reverse + lappend apply_cmd --reverse --cached if {[string index $mi 0] ne {M}} { unlock_index return } } else { - set failed_msg [mc "Failed to stage selected line."] - set to_context {-} + if {$revert} { + set failed_msg [mc "Failed to revert selected line."] + set to_context {+} + lappend apply_cmd --reverse + } else { + set failed_msg [mc "Failed to stage selected line."] + set to_context {-} + lappend apply_cmd --cached + } + if {[string index $mi 1] ne {M}} { unlock_index return } } + if {$revert} { + set query "[mc "Revert changes in file %s?" \ + [short_path $current_diff_path]] + +[mc "The selected lines will be permanently lost by the revert."]" + + set reply [revert_dialog $query] + if {$reply ne 1} { + unlock_index + return + } + } + set wholepatch {} while {$first_l < $last_l} { -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-19 21:41 ` [PATCH 2/3] git-gui: Add the ability to revert selected lines Pratyush Yadav @ 2019-08-20 19:21 ` Johannes Sixt 2019-08-20 19:29 ` Pratyush Yadav 0 siblings, 1 reply; 49+ messages in thread From: Johannes Sixt @ 2019-08-20 19:21 UTC (permalink / raw) To: Pratyush Yadav; +Cc: git, Junio C Hamano Am 19.08.19 um 23:41 schrieb Pratyush Yadav: > Just like the user can select lines to stage or unstage, add the > ability to revert selected lines. > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> > + if {$revert} { > + set query "[mc "Revert changes in file %s?" \ > + [short_path $current_diff_path]] > + > +[mc "The selected lines will be permanently lost by the revert."]" > + > + set reply [revert_dialog $query] Please don't do this. This confirmation dialog is unacceptable in my workflow. I use reversals of hunks and lines frequently, almost like a secondary code editor. My safety net is the undo function of the IDE, which works across reloads that are triggered by these external edits. These confirmations get in the way. > + if {$reply ne 1} { > + unlock_index > + return > + } > + } > + > set wholepatch {} > > while {$first_l < $last_l} { > -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-20 19:21 ` Johannes Sixt @ 2019-08-20 19:29 ` Pratyush Yadav 2019-08-20 21:19 ` Johannes Sixt 0 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-20 19:29 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, Junio C Hamano On 20/08/19 09:21PM, Johannes Sixt wrote: > Am 19.08.19 um 23:41 schrieb Pratyush Yadav: > > Just like the user can select lines to stage or unstage, add the > > ability to revert selected lines. > > > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> > > > + if {$revert} { > > + set query "[mc "Revert changes in file %s?" \ > > + [short_path $current_diff_path]] > > + > > +[mc "The selected lines will be permanently lost by the revert."]" > > + > > + set reply [revert_dialog $query] > > Please don't do this. This confirmation dialog is unacceptable in my > workflow. I use reversals of hunks and lines frequently, almost like a > secondary code editor. My safety net is the undo function of the IDE, > which works across reloads that are triggered by these external edits. > These confirmations get in the way. But not everyone uses an IDE. I use vim and it does not have any such undo feature that works across reloads. Not one I'm aware of anyway. It is absolutely necessary IMO to ask the user for confirmation before deleting their work, unless we have a built in safety net. So how about adding a config option that allows you to disable the confirmation dialog? Sounds like a reasonable compromise to me. > > + if {$reply ne 1} { > > + unlock_index > > + return > > + } > > + } > > + > > set wholepatch {} > > > > while {$first_l < $last_l} { > > > > -- Hannes -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-20 19:29 ` Pratyush Yadav @ 2019-08-20 21:19 ` Johannes Sixt 2019-08-21 21:48 ` Pratyush Yadav 0 siblings, 1 reply; 49+ messages in thread From: Johannes Sixt @ 2019-08-20 21:19 UTC (permalink / raw) To: Pratyush Yadav; +Cc: git, Junio C Hamano Am 20.08.19 um 21:29 schrieb Pratyush Yadav: > On 20/08/19 09:21PM, Johannes Sixt wrote: >> Please don't do this. This confirmation dialog is unacceptable in my >> workflow. I use reversals of hunks and lines frequently, almost like a >> secondary code editor. My safety net is the undo function of the IDE, >> which works across reloads that are triggered by these external edits. >> These confirmations get in the way. > > But not everyone uses an IDE. I use vim and it does not have any such > undo feature that works across reloads. Not one I'm aware of anyway. It > is absolutely necessary IMO to ask the user for confirmation before > deleting their work, unless we have a built in safety net. But you have a safety net built-in: Commit the work, then do the reversals in amend-mode. Now you can recover old state to your heart's content. That's recommended anyway if stuff is potentially precious. > So how about adding a config option that allows you to disable the > confirmation dialog? Sounds like a reasonable compromise to me. That's always an option. Needless to say that I'd prefer it off by default; I don't need three safety nets. -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-20 21:19 ` Johannes Sixt @ 2019-08-21 21:48 ` Pratyush Yadav 2019-08-23 13:01 ` Johannes Schindelin 0 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-21 21:48 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, Junio C Hamano On 20/08/19 11:19PM, Johannes Sixt wrote: > Am 20.08.19 um 21:29 schrieb Pratyush Yadav: > > On 20/08/19 09:21PM, Johannes Sixt wrote: > >> Please don't do this. This confirmation dialog is unacceptable in my > >> workflow. I use reversals of hunks and lines frequently, almost like a > >> secondary code editor. My safety net is the undo function of the IDE, > >> which works across reloads that are triggered by these external edits. > >> These confirmations get in the way. > > > > But not everyone uses an IDE. I use vim and it does not have any such > > undo feature that works across reloads. Not one I'm aware of anyway. It > > is absolutely necessary IMO to ask the user for confirmation before > > deleting their work, unless we have a built in safety net. > > But you have a safety net built-in: Commit the work, then do the > reversals in amend-mode. Now you can recover old state to your heart's > content. That's recommended anyway if stuff is potentially precious. I suppose we disagree on this. I feel very uncomfortable removing the prompt by default, because it is pretty easy to mis-click revert instead of stage, and all of a sudden lots of your work is gone. It is a pretty common workflow to make some changes, stage some hunks in one commit and then some others in the next. Not everyone (including me) will first commit changes, then amend them, especially if they are not that big or complicated. Accidentally deleting your work, no matter how small, because of a misclick sucks. So, I feel strongly in favor of keeping the prompt on by default. I will add a config option to disable it for people who are willing to accept misclicks. That keeps both sides of the argument happy. You just have to disable it once in your global config and you're good to go. > > So how about adding a config option that allows you to disable the > > confirmation dialog? Sounds like a reasonable compromise to me. > > That's always an option. Needless to say that I'd prefer it off by > default; I don't need three safety nets. > > -- Hannes -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-21 21:48 ` Pratyush Yadav @ 2019-08-23 13:01 ` Johannes Schindelin 2019-08-23 16:28 ` Junio C Hamano 0 siblings, 1 reply; 49+ messages in thread From: Johannes Schindelin @ 2019-08-23 13:01 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Johannes Sixt, git, Junio C Hamano Hi, On Thu, 22 Aug 2019, Pratyush Yadav wrote: > On 20/08/19 11:19PM, Johannes Sixt wrote: > > Am 20.08.19 um 21:29 schrieb Pratyush Yadav: > > > On 20/08/19 09:21PM, Johannes Sixt wrote: > > >> Please don't do this. This confirmation dialog is unacceptable in my > > >> workflow. I use reversals of hunks and lines frequently, almost like a > > >> secondary code editor. My safety net is the undo function of the IDE, > > >> which works across reloads that are triggered by these external edits. > > >> These confirmations get in the way. > > > > > > But not everyone uses an IDE. I use vim and it does not have any such > > > undo feature that works across reloads. Not one I'm aware of anyway. It > > > is absolutely necessary IMO to ask the user for confirmation before > > > deleting their work, unless we have a built in safety net. > > > > But you have a safety net built-in: Commit the work, then do the > > reversals in amend-mode. Now you can recover old state to your heart's > > content. That's recommended anyway if stuff is potentially precious. > > I suppose we disagree on this. I feel very uncomfortable removing the > prompt by default, because it is pretty easy to mis-click revert instead > of stage, and all of a sudden lots of your work is gone. It is a pretty > common workflow to make some changes, stage some hunks in one commit and > then some others in the next. Not everyone (including me) will first > commit changes, then amend them, especially if they are not that big or > complicated. Accidentally deleting your work, no matter how small, > because of a misclick sucks. > > So, I feel strongly in favor of keeping the prompt on by default. I will > add a config option to disable it for people who are willing to accept > misclicks. That keeps both sides of the argument happy. You just have to > disable it once in your global config and you're good to go. Maybe the direction taken by this discussion merely suggests that the design is a bit unfortunate. Why "revert"? Why not "stash" instead? Then you don't need to have that annoying confirmation dialog. Ciao, Johannes > > > > So how about adding a config option that allows you to disable the > > > confirmation dialog? Sounds like a reasonable compromise to me. > > > > That's always an option. Needless to say that I'd prefer it off by > > default; I don't need three safety nets. > > > > -- Hannes > > -- > Regards, > Pratyush Yadav > ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-23 13:01 ` Johannes Schindelin @ 2019-08-23 16:28 ` Junio C Hamano 2019-08-23 17:03 ` Pratyush Yadav 0 siblings, 1 reply; 49+ messages in thread From: Junio C Hamano @ 2019-08-23 16:28 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Pratyush Yadav, Johannes Sixt, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Maybe the direction taken by this discussion merely suggests that the > design is a bit unfortunate. Why "revert"? Why not "stash" instead? Then > you don't need to have that annoying confirmation dialog. Interesting, but it would need a bit more tweak than a simple "stash" for it to be truly helpful, I would imagine. The primary purpose of stashing from end user's point of view is to save some changes, that are not immediately usable in the context of the corrent work, away, so that they can be retrieved later, with a side effect of wiping the stashed changes from the working tree. The command could be (re|ab)used to wipe local changes you have in the working tree, but that would accumulate cruft that you most of the time (unless you make a mistake) wanted to discard and wanted to never look at again in the stash. If done using the same 'stash' ref that is used by the normal "git stash", the stash entries created by "git stash" because the user really wanted to keep them for later use would be drowned among these "kept just in case" stash entries that were created as a side effect of (ab)using stash in place of "restore". But "git stash" can be told to use a different ref, so perhaps by using a separate one for this "just in case" purpose, with the expectation that the entries are almost always safe to discard unless the user made a mistake and take it back immediately (i.e. "undo"), it would not hurt the normal use of "git stash" *and* give the "revert" necessary safety valve at the same time. Thanks. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-23 16:28 ` Junio C Hamano @ 2019-08-23 17:03 ` Pratyush Yadav 2019-08-23 19:17 ` Johannes Sixt 0 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-23 17:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git, Bert Wesarg +Cc Bert. This has the suggestion I was talking about in one of my previous replies. On 23/08/19 09:28AM, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > Maybe the direction taken by this discussion merely suggests that the > > design is a bit unfortunate. Why "revert"? Why not "stash" instead? Then > > you don't need to have that annoying confirmation dialog. > > Interesting, but it would need a bit more tweak than a simple > "stash" for it to be truly helpful, I would imagine. > > The primary purpose of stashing from end user's point of view is to > save some changes, that are not immediately usable in the context of > the corrent work, away, so that they can be retrieved later, with a > side effect of wiping the stashed changes from the working tree. The > command could be (re|ab)used to wipe local changes you have in the > working tree, but that would accumulate cruft that you most of the > time (unless you make a mistake) wanted to discard and wanted to > never look at again in the stash. If done using the same 'stash' ref > that is used by the normal "git stash", the stash entries created by > "git stash" because the user really wanted to keep them for later > use would be drowned among these "kept just in case" stash entries > that were created as a side effect of (ab)using stash in place of > "restore". Thank you. I was just about to write exactly this. > But "git stash" can be told to use a different ref, so perhaps by > using a separate one for this "just in case" purpose, with the > expectation that the entries are almost always safe to discard > unless the user made a mistake and take it back immediately > (i.e. "undo"), it would not hurt the normal use of "git stash" *and* > give the "revert" necessary safety valve at the same time. I propose a simpler solution. Essentially how the revert works is it generates a diff and stores that in a variable. It then calls git-apply with --reverse passed in case of reverts and unstages, and without the --reverse in case of staging, and then feeds git-apply the generated diff. So how about we keep a copy of the diff in another variable. This allows us to enable undoing of reverts. The obvious limitations are that firstly, unless we use a stack/deque that means only one undo is allowed. I'm not sure if using an undo stack would be worth the added complexity. Secondly, if the working tree is changed between the revert and the undo, there are chances of a merge conflict. If people are okay with these limitations, we can be rid of the confirmation dialog while providing a safety net. Sounds like a good idea? -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 2/3] git-gui: Add the ability to revert selected lines 2019-08-23 17:03 ` Pratyush Yadav @ 2019-08-23 19:17 ` Johannes Sixt 0 siblings, 0 replies; 49+ messages in thread From: Johannes Sixt @ 2019-08-23 19:17 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Junio C Hamano, Johannes Schindelin, git, Bert Wesarg Am 23.08.19 um 19:03 schrieb Pratyush Yadav: > So how about we keep a copy of the diff in another variable. This allows > us to enable undoing of reverts. The obvious limitations are that > firstly, unless we use a stack/deque that means only one undo is > allowed. I'm not sure if using an undo stack would be worth the added > complexity. Secondly, if the working tree is changed between the revert > and the undo, there are chances of a merge conflict. > > If people are okay with these limitations, we can be rid of the > confirmation dialog while providing a safety net. Sounds like a good > idea? Yes, sounds like an idea worth persuing. -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 3/3] git-gui: Add the ability to revert selected hunk 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav 2019-08-19 21:41 ` [PATCH 1/3] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav 2019-08-19 21:41 ` [PATCH 2/3] git-gui: Add the ability to revert selected lines Pratyush Yadav @ 2019-08-19 21:41 ` Pratyush Yadav 2019-08-20 18:47 ` [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Junio C Hamano ` (3 subsequent siblings) 6 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-19 21:41 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano Just like the user can select a hunk to stage or unstage, add the ability to revert hunks. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui/git-gui.sh | 14 +++++++++++++- git-gui/lib/diff.tcl | 34 +++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh index 2011894bef..cfa682ff59 100755 --- a/git-gui/git-gui.sh +++ b/git-gui/git-gui.sh @@ -3606,9 +3606,14 @@ set ctxm .vpane.lower.diff.body.ctxm menu $ctxm -tearoff 0 $ctxm add command \ -label [mc "Apply/Reverse Hunk"] \ - -command {apply_hunk $cursorX $cursorY} + -command {apply_or_revert_hunk $cursorX $cursorY 0} set ui_diff_applyhunk [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] +$ctxm add command \ + -label [mc "Revert Hunk"] \ + -command {apply_or_revert_hunk $cursorX $cursorY 1} +set ui_diff_reverthunk [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_reverthunk -state] $ctxm add command \ -label [mc "Apply/Reverse Line"] \ -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} @@ -3715,6 +3720,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}] if {$::ui_index eq $::current_diff_side} { set l [mc "Unstage Hunk From Commit"] + set h [mc "Revert Hunk"] + if {$has_range} { set t [mc "Unstage Lines From Commit"] set r [mc "Revert Lines"] @@ -3724,6 +3731,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { } } else { set l [mc "Stage Hunk For Commit"] + set h [mc "Revert Hunk"] + if {$has_range} { set t [mc "Stage Lines For Commit"] set r [mc "Revert Lines"] @@ -3758,6 +3767,9 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { $ctxm entryconf $::ui_diff_applyline -state $s -label $t $ctxm entryconf $::ui_diff_revertline -state $revert_state \ -label $r + $ctxm entryconf $::ui_diff_reverthunk -state $revert_state \ + -label $h + tk_popup $ctxm $X $Y } } diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl index 4b2b00df4b..a818e68dad 100644 --- a/git-gui/lib/diff.tcl +++ b/git-gui/lib/diff.tcl @@ -567,30 +567,50 @@ proc read_diff {fd conflict_size cont_info} { } } -proc apply_hunk {x y} { +proc apply_or_revert_hunk {x y revert} { global current_diff_path current_diff_header current_diff_side global ui_diff ui_index file_states if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return - set apply_cmd {apply --cached --whitespace=nowarn} + set apply_cmd {apply --whitespace=nowarn} set mi [lindex $file_states($current_diff_path) 0] if {$current_diff_side eq $ui_index} { set failed_msg [mc "Failed to unstage selected hunk."] - lappend apply_cmd --reverse + lappend apply_cmd --reverse --cached if {[string index $mi 0] ne {M}} { unlock_index return } } else { - set failed_msg [mc "Failed to stage selected hunk."] + if {$revert} { + set failed_msg [mc "Failed to revert selected hunk."] + lappend apply_cmd --reverse + } else { + set failed_msg [mc "Failed to stage selected hunk."] + lappend apply_cmd --cached + } + if {[string index $mi 1] ne {M}} { unlock_index return } } + if {$revert} { + set query "[mc "Revert changes in file %s?" \ + [short_path $current_diff_path]] + +[mc "The selected hunk will be permanently lost by the revert."]" + + set reply [revert_dialog $query] + if {$reply ne 1} { + unlock_index + return + } + } + set s_lno [lindex [split [$ui_diff index @$x,$y] .] 0] set s_lno [$ui_diff search -backwards -regexp ^@@ $s_lno.0 0.0] if {$s_lno eq {}} { @@ -619,13 +639,17 @@ proc apply_hunk {x y} { $ui_diff delete $s_lno $e_lno $ui_diff conf -state disabled + # Check if the hunk was the last one in the file. if {[$ui_diff get 1.0 end] eq "\n"} { set o _ } else { set o ? } - if {$current_diff_side eq $ui_index} { + # Update the status flags. + if {$revert} { + set mi [string index $mi 0]$o + } elseif {$current_diff_side eq $ui_index} { set mi ${o}M } elseif {[string index $mi 0] eq {_}} { set mi M$o -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav ` (2 preceding siblings ...) 2019-08-19 21:41 ` [PATCH 3/3] git-gui: Add the ability to revert selected hunk Pratyush Yadav @ 2019-08-20 18:47 ` Junio C Hamano 2019-08-20 19:49 ` Pratyush Yadav 2019-08-21 7:06 ` Bert Wesarg ` (2 subsequent siblings) 6 siblings, 1 reply; 49+ messages in thread From: Junio C Hamano @ 2019-08-20 18:47 UTC (permalink / raw) To: Pratyush Yadav; +Cc: git Pratyush Yadav <me@yadavpratyush.com> writes: > This series adds the ability to revert selected lines and hunks in > git-gui. Partially based on the patch by Bert Wesarg [0]. > > The commits can be found in the topic branch 'py/git-gui-revert-lines' > at https://github.com/prati0100/git/tree/py/git-gui-revert-lines Please don't do this. I would strongly prefer keeping the contination of history from the history in Pat's git-gui repository. If you clone from git://repo.or.cz/git-gui.git/ you'll notice everything for git-gui is one level up, and nothing for git-core is duplicated in there. You'll work on top of that, so the patches to the git-gui project should not say things like --- git-gui/lib/index.tcl | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl index b588db11d9..cb7f74af45 100644 ... The leading "git-gui/" should not appear. I have a fork of Pat's history with a single topic on top at https://github.com/gitster/git-gui/ so building on top would maintain the continuity of the history as well. Once you prepared your changes in such a clone of the git-gui project, in order to test them with the rest of Git, you'd make a trial merge with the -Xsubtree=git-gui option. Perhaps you have git-gui's clone in $HOME/git-gui and git's clone in $HOME/git, like so $ cd $HOME $ git clone https://github.com/gitster/git-gui git-gui $ cd git-gui ... from now on, you'd work on git-gui in this directory ... ... do the work of this topic perhaps on 'revert-hunks' branch ... $ git clone https://github.com/gitster/git git $ cd ../git ... trial integration ... $ git pull -Xsubtree=git-gui ../git-gui/ revert-hunks ... do whatever testing necessary ... As an interim (and hopefully evantual) maintainer of the git-gui project, you'd publish from your local git-gui directory to a fork of git-gui project you host somewhere. Your patches for review would also be taken from your local git-gui directory. Thanks. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines 2019-08-20 18:47 ` [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Junio C Hamano @ 2019-08-20 19:49 ` Pratyush Yadav 0 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-20 19:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 20/08/19 11:47AM, Junio C Hamano wrote: > Pratyush Yadav <me@yadavpratyush.com> writes: > > > This series adds the ability to revert selected lines and hunks in > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > The commits can be found in the topic branch 'py/git-gui-revert-lines' > > at https://github.com/prati0100/git/tree/py/git-gui-revert-lines > > Please don't do this. All right, I'll send a re-roll of the patches based on your fork of Pat's tree as soon as I get some time. PS: Thanks for the detailed explanation of how I should structure my workflow :) > I would strongly prefer keeping the contination of history from the > history in Pat's git-gui repository. If you clone from > > git://repo.or.cz/git-gui.git/ > > you'll notice everything for git-gui is one level up, and nothing > for git-core is duplicated in there. You'll work on top of that, so > the patches to the git-gui project should not say things like > > --- > git-gui/lib/index.tcl | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl > index b588db11d9..cb7f74af45 100644 > ... > > The leading "git-gui/" should not appear. > > I have a fork of Pat's history with a single topic on top at > https://github.com/gitster/git-gui/ so building on top would > maintain the continuity of the history as well. > > Once you prepared your changes in such a clone of the git-gui > project, in order to test them with the rest of Git, you'd make a > trial merge with the -Xsubtree=git-gui option. Perhaps you have > git-gui's clone in $HOME/git-gui and git's clone in $HOME/git, like > so > > $ cd $HOME > $ git clone https://github.com/gitster/git-gui git-gui > $ cd git-gui > ... from now on, you'd work on git-gui in this directory ... > ... do the work of this topic perhaps on 'revert-hunks' branch ... > > $ git clone https://github.com/gitster/git git > $ cd ../git > ... trial integration ... > $ git pull -Xsubtree=git-gui ../git-gui/ revert-hunks > ... do whatever testing necessary ... > > As an interim (and hopefully evantual) maintainer of the git-gui > project, you'd publish from your local git-gui directory to a fork > of git-gui project you host somewhere. Your patches for review > would also be taken from your local git-gui directory. > > Thanks. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav ` (3 preceding siblings ...) 2019-08-20 18:47 ` [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Junio C Hamano @ 2019-08-21 7:06 ` Bert Wesarg 2019-08-21 21:30 ` Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav 6 siblings, 1 reply; 49+ messages in thread From: Bert Wesarg @ 2019-08-21 7:06 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git Mailing List, Junio C Hamano Dear Pratyush, putting me in Cc would have been nice :( I will look into your patches in the comming hours. Bert On Mon, Aug 19, 2019 at 11:41 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Hi, > > This series adds the ability to revert selected lines and hunks in > git-gui. Partially based on the patch by Bert Wesarg [0]. > > The commits can be found in the topic branch 'py/git-gui-revert-lines' > at https://github.com/prati0100/git/tree/py/git-gui-revert-lines > > Once reviewed, pull the commits from > 832064f93d3ad432616e96ca70f682a7cfdcc3e0 till > 72eed27a29f1e71cbefefa6b19f96c89793ac74d. > > Let me know if there is some other way I'm supposed to ask for a pull. > > [0] > https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wesarg@googlemail.com/ > > Pratyush Yadav (3): > git-gui: Move revert confirmation dialog creation to separate function > git-gui: Add the ability to revert selected lines > git-gui: Add the ability to revert selected hunk > > git-gui/git-gui.sh | 39 ++++++++++++++++++++++++-- > git-gui/lib/diff.tcl | 65 ++++++++++++++++++++++++++++++++++++------- > git-gui/lib/index.tcl | 27 +++++++++++------- > 3 files changed, 109 insertions(+), 22 deletions(-) > > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines 2019-08-21 7:06 ` Bert Wesarg @ 2019-08-21 21:30 ` Pratyush Yadav 0 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-21 21:30 UTC (permalink / raw) To: Bert Wesarg; +Cc: Git Mailing List, Junio C Hamano On 21/08/19 09:06AM, Bert Wesarg wrote: > Dear Pratyush, > > putting me in Cc would have been nice :( I wasn't sure if you were interested in git-gui any more, so I decided to not bother you. I will Cc you when I send a re-roll. > I will look into your patches in the comming hours. Thanks. -- Regards, Pratyush Yadav > Bert > > On Mon, Aug 19, 2019 at 11:41 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > > > Hi, > > > > This series adds the ability to revert selected lines and hunks in > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > The commits can be found in the topic branch 'py/git-gui-revert-lines' > > at https://github.com/prati0100/git/tree/py/git-gui-revert-lines > > > > Once reviewed, pull the commits from > > 832064f93d3ad432616e96ca70f682a7cfdcc3e0 till > > 72eed27a29f1e71cbefefa6b19f96c89793ac74d. > > > > Let me know if there is some other way I'm supposed to ask for a pull. > > > > [0] > > https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wesarg@googlemail.com/ > > > > Pratyush Yadav (3): > > git-gui: Move revert confirmation dialog creation to separate function > > git-gui: Add the ability to revert selected lines > > git-gui: Add the ability to revert selected hunk > > > > git-gui/git-gui.sh | 39 ++++++++++++++++++++++++-- > > git-gui/lib/diff.tcl | 65 ++++++++++++++++++++++++++++++++++++------- > > git-gui/lib/index.tcl | 27 +++++++++++------- > > 3 files changed, 109 insertions(+), 22 deletions(-) > > > > -- > > 2.21.0 > > ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav ` (4 preceding siblings ...) 2019-08-21 7:06 ` Bert Wesarg @ 2019-08-22 22:01 ` Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 1/4] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav ` (5 more replies) 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav 6 siblings, 6 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-22 22:01 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Hi, This series adds the ability to revert selected lines and hunks in git-gui. Partially based on the patch by Bert Wesarg [0]. The commits can be found in the topic branch 'py/revert-hunks-lines' at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines Once reviewed, pull the commits from 415ce3f8582769d1d454b3796dc6c9c847cefa87 till 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and apply them locally, whichever you prefer. Changes in v2: - Add an option to disable the revert confirmation prompt as suggested by Johannes Sixt. - Base the patches on Pat's git-gui tree instead of git.git. [0] https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wesarg@googlemail.com/ Pratyush Yadav (4): git-gui: Move revert confirmation dialog creation to separate function git-gui: Add option to disable the revert confirmation prompt git-gui: Add the ability to revert selected lines git-gui: Add the ability to revert selected hunk git-gui.sh | 40 +++++++++++++++++++++++++++++-- lib/diff.tcl | 65 ++++++++++++++++++++++++++++++++++++++++++-------- lib/index.tcl | 31 ++++++++++++++++-------- lib/option.tcl | 1 + 4 files changed, 115 insertions(+), 22 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 1/4] git-gui: Move revert confirmation dialog creation to separate function 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav @ 2019-08-22 22:01 ` Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 2/4] git-gui: Add option to disable the revert confirmation prompt Pratyush Yadav ` (4 subsequent siblings) 5 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-22 22:01 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Upcoming commits will introduce reverting lines and hunks. They also need to prompt the user for confirmation. Put the dialog creation in its separate function so the same code won't be repeated again and again. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- lib/index.tcl | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/index.tcl b/lib/index.tcl index b588db1..cb7f74a 100644 --- a/lib/index.tcl +++ b/lib/index.tcl @@ -388,6 +388,18 @@ proc do_add_all {} { add_helper [mc "Adding all changed files"] $paths } +proc revert_dialog {query} { + return [tk_dialog \ + .confirm_revert \ + "[appname] ([reponame])" \ + "$query" \ + question \ + 1 \ + [mc "Do Nothing"] \ + [mc "Revert Changes"] \ + ] +} + proc revert_helper {txt paths} { global file_states current_diff_path @@ -430,17 +442,12 @@ proc revert_helper {txt paths} { set query [mc "Revert changes in these %i files?" $n] } - set reply [tk_dialog \ - .confirm_revert \ - "[appname] ([reponame])" \ - "$query + set query "$query + +[mc "Any unstaged changes will be permanently lost by the revert."]" + + set reply [revert_dialog $query] -[mc "Any unstaged changes will be permanently lost by the revert."]" \ - question \ - 1 \ - [mc "Do Nothing"] \ - [mc "Revert Changes"] \ - ] if {$reply == 1} { checkout_index \ $txt \ -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 2/4] git-gui: Add option to disable the revert confirmation prompt 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 1/4] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav @ 2019-08-22 22:01 ` Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 3/4] git-gui: Add the ability to revert selected lines Pratyush Yadav ` (3 subsequent siblings) 5 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-22 22:01 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg When reverting files (or hunks or lines that future commits will add), a confirmation dialog is shown to make sure the user actually wanted to revert, and did not accidentally click revert. But for some people's workflow this is an hindrance. So add an option to disable this behaviour for people who are comfortable risking accidental reverts. The default behaviour is to ask for confirmation. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui.sh | 1 + lib/index.tcl | 22 +++++++++++++--------- lib/option.tcl | 1 + 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 5bc21b8..b7811d8 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -903,6 +903,7 @@ set font_descs { } set default_config(gui.stageuntracked) ask set default_config(gui.displayuntracked) true +set default_config(gui.revertaskconfirmation) true ###################################################################### ## diff --git a/lib/index.tcl b/lib/index.tcl index cb7f74a..dd694d1 100644 --- a/lib/index.tcl +++ b/lib/index.tcl @@ -389,15 +389,19 @@ proc do_add_all {} { } proc revert_dialog {query} { - return [tk_dialog \ - .confirm_revert \ - "[appname] ([reponame])" \ - "$query" \ - question \ - 1 \ - [mc "Do Nothing"] \ - [mc "Revert Changes"] \ - ] + if {[is_config_true gui.revertaskconfirmation]} { + return [tk_dialog \ + .confirm_revert \ + "[appname] ([reponame])" \ + "$query" \ + question \ + 1 \ + [mc "Do Nothing"] \ + [mc "Revert Changes"] \ + ] + } else { + return 1 + } } proc revert_helper {txt paths} { diff --git a/lib/option.tcl b/lib/option.tcl index e43971b..cb62d5d 100644 --- a/lib/option.tcl +++ b/lib/option.tcl @@ -162,6 +162,7 @@ proc do_options {} { {s gui.stageuntracked {mc "Staging of untracked files"} {list "yes" "no" "ask"}} {b gui.displayuntracked {mc "Show untracked files"}} {i-1..99 gui.tabsize {mc "Tab spacing"}} + {b gui.revertaskconfirmation {mc "Ask for confirmation when reverting changes"}} } { set type [lindex $option 0] set name [lindex $option 1] -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v2 3/4] git-gui: Add the ability to revert selected lines 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 1/4] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 2/4] git-gui: Add option to disable the revert confirmation prompt Pratyush Yadav @ 2019-08-22 22:01 ` Pratyush Yadav 2019-08-23 6:29 ` Bert Wesarg 2019-08-22 22:01 ` [PATCH v2 4/4] git-gui: Add the ability to revert selected hunk Pratyush Yadav ` (2 subsequent siblings) 5 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-22 22:01 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Just like the user can select lines to stage or unstage, add the ability to revert selected lines. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui.sh | 25 ++++++++++++++++++++++++- lib/diff.tcl | 31 ++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index b7811d8..9d84ba9 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -3588,9 +3588,15 @@ set ui_diff_applyhunk [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] $ctxm add command \ -label [mc "Apply/Reverse Line"] \ - -command {apply_range_or_line $cursorX $cursorY; do_rescan} + -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} set ui_diff_applyline [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state] +$ctxm add command \ + -label [mc "Revert Line"] \ + -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} +set ui_diff_revertline [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] +set ui_diff_revertline [$ctxm index last] $ctxm add separator $ctxm add command \ -label [mc "Show Less Context"] \ @@ -3688,15 +3694,19 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { set l [mc "Unstage Hunk From Commit"] if {$has_range} { set t [mc "Unstage Lines From Commit"] + set r [mc "Revert Lines"] } else { set t [mc "Unstage Line From Commit"] + set r [mc "Revert Line"] } } else { set l [mc "Stage Hunk For Commit"] if {$has_range} { set t [mc "Stage Lines For Commit"] + set r [mc "Revert Lines"] } else { set t [mc "Stage Line For Commit"] + set r [mc "Revert Line"] } } if {$::is_3way_diff @@ -3707,11 +3717,24 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { || [string match {T?} $state] || [has_textconv $current_diff_path]} { set s disabled + set revert_state disabled } else { set s normal + + # Only allow reverting changes in the working tree. If + # the user wants to revert changes in the index, they + # need to unstage those first. + if {$::ui_workdir eq $::current_diff_side} { + set revert_state normal + } else { + set revert_state disabled + } } + $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l $ctxm entryconf $::ui_diff_applyline -state $s -label $t + $ctxm entryconf $::ui_diff_revertline -state $revert_state \ + -label $r tk_popup $ctxm $X $Y } } diff --git a/lib/diff.tcl b/lib/diff.tcl index 4cae10a..00b15f5 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -640,7 +640,7 @@ proc apply_hunk {x y} { } } -proc apply_range_or_line {x y} { +proc apply_or_revert_range_or_line {x y revert} { global current_diff_path current_diff_header current_diff_side global ui_diff ui_index file_states @@ -660,25 +660,46 @@ proc apply_range_or_line {x y} { if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return - set apply_cmd {apply --cached --whitespace=nowarn} + set apply_cmd {apply --whitespace=nowarn} set mi [lindex $file_states($current_diff_path) 0] if {$current_diff_side eq $ui_index} { set failed_msg [mc "Failed to unstage selected line."] set to_context {+} - lappend apply_cmd --reverse + lappend apply_cmd --reverse --cached if {[string index $mi 0] ne {M}} { unlock_index return } } else { - set failed_msg [mc "Failed to stage selected line."] - set to_context {-} + if {$revert} { + set failed_msg [mc "Failed to revert selected line."] + set to_context {+} + lappend apply_cmd --reverse + } else { + set failed_msg [mc "Failed to stage selected line."] + set to_context {-} + lappend apply_cmd --cached + } + if {[string index $mi 1] ne {M}} { unlock_index return } } + if {$revert} { + set query "[mc "Revert changes in file %s?" \ + [short_path $current_diff_path]] + +[mc "The selected lines will be permanently lost by the revert."]" + + set reply [revert_dialog $query] + if {$reply ne 1} { + unlock_index + return + } + } + set wholepatch {} while {$first_l < $last_l} { -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v2 3/4] git-gui: Add the ability to revert selected lines 2019-08-22 22:01 ` [PATCH v2 3/4] git-gui: Add the ability to revert selected lines Pratyush Yadav @ 2019-08-23 6:29 ` Bert Wesarg 2019-08-23 16:51 ` Pratyush Yadav 0 siblings, 1 reply; 49+ messages in thread From: Bert Wesarg @ 2019-08-23 6:29 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt On Fri, Aug 23, 2019 at 12:01 AM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Just like the user can select lines to stage or unstage, add the > ability to revert selected lines. > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> > --- > git-gui.sh | 25 ++++++++++++++++++++++++- > lib/diff.tcl | 31 ++++++++++++++++++++++++++----- > 2 files changed, 50 insertions(+), 6 deletions(-) > > diff --git a/git-gui.sh b/git-gui.sh > index b7811d8..9d84ba9 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -3588,9 +3588,15 @@ set ui_diff_applyhunk [$ctxm index last] > lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] > $ctxm add command \ > -label [mc "Apply/Reverse Line"] \ > - -command {apply_range_or_line $cursorX $cursorY; do_rescan} > + -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} > set ui_diff_applyline [$ctxm index last] > lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state] > +$ctxm add command \ > + -label [mc "Revert Line"] \ > + -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} > +set ui_diff_revertline [$ctxm index last] > +lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] > +set ui_diff_revertline [$ctxm index last] > $ctxm add separator > $ctxm add command \ > -label [mc "Show Less Context"] \ > @@ -3688,15 +3694,19 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > set l [mc "Unstage Hunk From Commit"] > if {$has_range} { > set t [mc "Unstage Lines From Commit"] > + set r [mc "Revert Lines"] > } else { > set t [mc "Unstage Line From Commit"] > + set r [mc "Revert Line"] > } > } else { > set l [mc "Stage Hunk For Commit"] > if {$has_range} { > set t [mc "Stage Lines For Commit"] > + set r [mc "Revert Lines"] > } else { > set t [mc "Stage Line For Commit"] > + set r [mc "Revert Line"] > } > } > if {$::is_3way_diff > @@ -3707,11 +3717,24 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > || [string match {T?} $state] > || [has_textconv $current_diff_path]} { > set s disabled > + set revert_state disabled > } else { > set s normal > + > + # Only allow reverting changes in the working tree. If > + # the user wants to revert changes in the index, they > + # need to unstage those first. > + if {$::ui_workdir eq $::current_diff_side} { > + set revert_state normal > + } else { > + set revert_state disabled > + } > } > + > $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l > $ctxm entryconf $::ui_diff_applyline -state $s -label $t > + $ctxm entryconf $::ui_diff_revertline -state $revert_state \ > + -label $r so you have the 'Revert Line(s)' menu entry always in the context menu, also when the context menu was opened for an staged file (than the menu entry is only disabled)? I think this is a step backwards from my original patch (which isn't mentioned/referenced at all in this patch anyway). My orignal patch also had this hunk in lib/diff.tcl: @@ -614,6 +619,8 @@ proc apply_hunk {x y} { if {$current_diff_side eq $ui_index} { set mi ${o}M + } elseif {$revert} { + set mi "[string index $mi 0]$o" } elseif {[string index $mi 0] eq {_}} { set mi M$o } else { which is missing here. I cannot remember why I added this here. But maybe you can? Best, Bert > tk_popup $ctxm $X $Y > } > } > diff --git a/lib/diff.tcl b/lib/diff.tcl > index 4cae10a..00b15f5 100644 > --- a/lib/diff.tcl > +++ b/lib/diff.tcl > @@ -640,7 +640,7 @@ proc apply_hunk {x y} { > } > } > > -proc apply_range_or_line {x y} { > +proc apply_or_revert_range_or_line {x y revert} { > global current_diff_path current_diff_header current_diff_side > global ui_diff ui_index file_states > > @@ -660,25 +660,46 @@ proc apply_range_or_line {x y} { > if {$current_diff_path eq {} || $current_diff_header eq {}} return > if {![lock_index apply_hunk]} return > > - set apply_cmd {apply --cached --whitespace=nowarn} > + set apply_cmd {apply --whitespace=nowarn} > set mi [lindex $file_states($current_diff_path) 0] > if {$current_diff_side eq $ui_index} { > set failed_msg [mc "Failed to unstage selected line."] > set to_context {+} > - lappend apply_cmd --reverse > + lappend apply_cmd --reverse --cached > if {[string index $mi 0] ne {M}} { > unlock_index > return > } > } else { > - set failed_msg [mc "Failed to stage selected line."] > - set to_context {-} > + if {$revert} { > + set failed_msg [mc "Failed to revert selected line."] > + set to_context {+} > + lappend apply_cmd --reverse > + } else { > + set failed_msg [mc "Failed to stage selected line."] > + set to_context {-} > + lappend apply_cmd --cached > + } > + > if {[string index $mi 1] ne {M}} { > unlock_index > return > } > } > > + if {$revert} { > + set query "[mc "Revert changes in file %s?" \ > + [short_path $current_diff_path]] > + > +[mc "The selected lines will be permanently lost by the revert."]" > + > + set reply [revert_dialog $query] > + if {$reply ne 1} { > + unlock_index > + return > + } > + } > + > set wholepatch {} > > while {$first_l < $last_l} { > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 3/4] git-gui: Add the ability to revert selected lines 2019-08-23 6:29 ` Bert Wesarg @ 2019-08-23 16:51 ` Pratyush Yadav 0 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-23 16:51 UTC (permalink / raw) To: Bert Wesarg; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt On 23/08/19 08:29AM, Bert Wesarg wrote: > On Fri, Aug 23, 2019 at 12:01 AM Pratyush Yadav <me@yadavpratyush.com> wrote: > > > > Just like the user can select lines to stage or unstage, add the > > ability to revert selected lines. > > > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> > > --- > > git-gui.sh | 25 ++++++++++++++++++++++++- > > lib/diff.tcl | 31 ++++++++++++++++++++++++++----- > > 2 files changed, 50 insertions(+), 6 deletions(-) > > > > diff --git a/git-gui.sh b/git-gui.sh > > index b7811d8..9d84ba9 100755 > > --- a/git-gui.sh > > +++ b/git-gui.sh > > @@ -3588,9 +3588,15 @@ set ui_diff_applyhunk [$ctxm index last] > > lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] > > $ctxm add command \ > > -label [mc "Apply/Reverse Line"] \ > > - -command {apply_range_or_line $cursorX $cursorY; do_rescan} > > + -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} > > set ui_diff_applyline [$ctxm index last] > > lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state] > > +$ctxm add command \ > > + -label [mc "Revert Line"] \ > > + -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} > > +set ui_diff_revertline [$ctxm index last] > > +lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] > > +set ui_diff_revertline [$ctxm index last] > > $ctxm add separator > > $ctxm add command \ > > -label [mc "Show Less Context"] \ > > @@ -3688,15 +3694,19 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > > set l [mc "Unstage Hunk From Commit"] > > if {$has_range} { > > set t [mc "Unstage Lines From Commit"] > > + set r [mc "Revert Lines"] > > } else { > > set t [mc "Unstage Line From Commit"] > > + set r [mc "Revert Line"] > > } > > } else { > > set l [mc "Stage Hunk For Commit"] > > if {$has_range} { > > set t [mc "Stage Lines For Commit"] > > + set r [mc "Revert Lines"] > > } else { > > set t [mc "Stage Line For Commit"] > > + set r [mc "Revert Line"] > > } > > } > > if {$::is_3way_diff > > @@ -3707,11 +3717,24 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > > || [string match {T?} $state] > > || [has_textconv $current_diff_path]} { > > set s disabled > > + set revert_state disabled > > } else { > > set s normal > > + > > + # Only allow reverting changes in the working tree. If > > + # the user wants to revert changes in the index, they > > + # need to unstage those first. > > + if {$::ui_workdir eq $::current_diff_side} { > > + set revert_state normal > > + } else { > > + set revert_state disabled > > + } > > } > > + > > $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l > > $ctxm entryconf $::ui_diff_applyline -state $s -label $t > > + $ctxm entryconf $::ui_diff_revertline -state $revert_state \ > > + -label $r > > so you have the 'Revert Line(s)' menu entry always in the context > menu, also when the context menu was opened for an staged file (than > the menu entry is only disabled)? I think this is a step backwards > from my original patch (which isn't mentioned/referenced at all in > this patch anyway). I intentionally did this because I thought keeping the menu layout the same at all times will help a bit with muscle memory. I don't mind your way either though. If you feel very strongly about this, I'll do it your way. > My orignal patch also had this hunk in lib/diff.tcl: > > @@ -614,6 +619,8 @@ proc apply_hunk {x y} { > > if {$current_diff_side eq $ui_index} { > set mi ${o}M > + } elseif {$revert} { > + set mi "[string index $mi 0]$o" > } elseif {[string index $mi 0] eq {_}} { > set mi M$o > } else { > > which is missing here. I cannot remember why I added this here. But > maybe you can? You are looking at the wrong patch. This patch is for reverting lines, not hunks. Since the current "Stage selected line(s)" function relies on a rescan to update file states, I went with that convention. The -command parameter for the menu entry triggers a rescan after the revert is done. The next patch deals with hunks, and it contains this if statement. > Best, > Bert > > > tk_popup $ctxm $X $Y > > } > > } > > diff --git a/lib/diff.tcl b/lib/diff.tcl > > index 4cae10a..00b15f5 100644 > > --- a/lib/diff.tcl > > +++ b/lib/diff.tcl > > @@ -640,7 +640,7 @@ proc apply_hunk {x y} { > > } > > } > > > > -proc apply_range_or_line {x y} { > > +proc apply_or_revert_range_or_line {x y revert} { > > global current_diff_path current_diff_header current_diff_side > > global ui_diff ui_index file_states > > > > @@ -660,25 +660,46 @@ proc apply_range_or_line {x y} { > > if {$current_diff_path eq {} || $current_diff_header eq {}} return > > if {![lock_index apply_hunk]} return > > > > - set apply_cmd {apply --cached --whitespace=nowarn} > > + set apply_cmd {apply --whitespace=nowarn} > > set mi [lindex $file_states($current_diff_path) 0] > > if {$current_diff_side eq $ui_index} { > > set failed_msg [mc "Failed to unstage selected line."] > > set to_context {+} > > - lappend apply_cmd --reverse > > + lappend apply_cmd --reverse --cached > > if {[string index $mi 0] ne {M}} { > > unlock_index > > return > > } > > } else { > > - set failed_msg [mc "Failed to stage selected line."] > > - set to_context {-} > > + if {$revert} { > > + set failed_msg [mc "Failed to revert selected line."] > > + set to_context {+} > > + lappend apply_cmd --reverse > > + } else { > > + set failed_msg [mc "Failed to stage selected line."] > > + set to_context {-} > > + lappend apply_cmd --cached > > + } > > + > > if {[string index $mi 1] ne {M}} { > > unlock_index > > return > > } > > } > > > > + if {$revert} { > > + set query "[mc "Revert changes in file %s?" \ > > + [short_path $current_diff_path]] > > + > > +[mc "The selected lines will be permanently lost by the revert."]" > > + > > + set reply [revert_dialog $query] > > + if {$reply ne 1} { > > + unlock_index > > + return > > + } > > + } > > + > > set wholepatch {} > > > > while {$first_l < $last_l} { > > -- > > 2.21.0 > > -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 4/4] git-gui: Add the ability to revert selected hunk 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav ` (2 preceding siblings ...) 2019-08-22 22:01 ` [PATCH v2 3/4] git-gui: Add the ability to revert selected lines Pratyush Yadav @ 2019-08-22 22:01 ` Pratyush Yadav 2019-08-22 22:34 ` [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines Junio C Hamano 2019-08-23 23:43 ` David Aguilar 5 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-22 22:01 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Just like the user can select a hunk to stage or unstage, add the ability to revert hunks. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui.sh | 14 +++++++++++++- lib/diff.tcl | 34 +++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 9d84ba9..13fef74 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -3583,9 +3583,14 @@ set ctxm .vpane.lower.diff.body.ctxm menu $ctxm -tearoff 0 $ctxm add command \ -label [mc "Apply/Reverse Hunk"] \ - -command {apply_hunk $cursorX $cursorY} + -command {apply_or_revert_hunk $cursorX $cursorY 0} set ui_diff_applyhunk [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] +$ctxm add command \ + -label [mc "Revert Hunk"] \ + -command {apply_or_revert_hunk $cursorX $cursorY 1} +set ui_diff_reverthunk [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_reverthunk -state] $ctxm add command \ -label [mc "Apply/Reverse Line"] \ -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} @@ -3692,6 +3697,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}] if {$::ui_index eq $::current_diff_side} { set l [mc "Unstage Hunk From Commit"] + set h [mc "Revert Hunk"] + if {$has_range} { set t [mc "Unstage Lines From Commit"] set r [mc "Revert Lines"] @@ -3701,6 +3708,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { } } else { set l [mc "Stage Hunk For Commit"] + set h [mc "Revert Hunk"] + if {$has_range} { set t [mc "Stage Lines For Commit"] set r [mc "Revert Lines"] @@ -3735,6 +3744,9 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { $ctxm entryconf $::ui_diff_applyline -state $s -label $t $ctxm entryconf $::ui_diff_revertline -state $revert_state \ -label $r + $ctxm entryconf $::ui_diff_reverthunk -state $revert_state \ + -label $h + tk_popup $ctxm $X $Y } } diff --git a/lib/diff.tcl b/lib/diff.tcl index 00b15f5..6a9c760 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -567,30 +567,50 @@ proc read_diff {fd conflict_size cont_info} { } } -proc apply_hunk {x y} { +proc apply_or_revert_hunk {x y revert} { global current_diff_path current_diff_header current_diff_side global ui_diff ui_index file_states if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return - set apply_cmd {apply --cached --whitespace=nowarn} + set apply_cmd {apply --whitespace=nowarn} set mi [lindex $file_states($current_diff_path) 0] if {$current_diff_side eq $ui_index} { set failed_msg [mc "Failed to unstage selected hunk."] - lappend apply_cmd --reverse + lappend apply_cmd --reverse --cached if {[string index $mi 0] ne {M}} { unlock_index return } } else { - set failed_msg [mc "Failed to stage selected hunk."] + if {$revert} { + set failed_msg [mc "Failed to revert selected hunk."] + lappend apply_cmd --reverse + } else { + set failed_msg [mc "Failed to stage selected hunk."] + lappend apply_cmd --cached + } + if {[string index $mi 1] ne {M}} { unlock_index return } } + if {$revert} { + set query "[mc "Revert changes in file %s?" \ + [short_path $current_diff_path]] + +[mc "The selected hunk will be permanently lost by the revert."]" + + set reply [revert_dialog $query] + if {$reply ne 1} { + unlock_index + return + } + } + set s_lno [lindex [split [$ui_diff index @$x,$y] .] 0] set s_lno [$ui_diff search -backwards -regexp ^@@ $s_lno.0 0.0] if {$s_lno eq {}} { @@ -619,13 +639,17 @@ proc apply_hunk {x y} { $ui_diff delete $s_lno $e_lno $ui_diff conf -state disabled + # Check if the hunk was the last one in the file. if {[$ui_diff get 1.0 end] eq "\n"} { set o _ } else { set o ? } - if {$current_diff_side eq $ui_index} { + # Update the status flags. + if {$revert} { + set mi [string index $mi 0]$o + } elseif {$current_diff_side eq $ui_index} { set mi ${o}M } elseif {[string index $mi 0] eq {_}} { set mi M$o -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav ` (3 preceding siblings ...) 2019-08-22 22:01 ` [PATCH v2 4/4] git-gui: Add the ability to revert selected hunk Pratyush Yadav @ 2019-08-22 22:34 ` Junio C Hamano 2019-08-22 22:51 ` Pratyush Yadav 2019-08-23 23:43 ` David Aguilar 5 siblings, 1 reply; 49+ messages in thread From: Junio C Hamano @ 2019-08-22 22:34 UTC (permalink / raw) To: Pratyush Yadav; +Cc: git, Johannes Sixt, Bert Wesarg Pratyush Yadav <me@yadavpratyush.com> writes: > This series adds the ability to revert selected lines and hunks in > git-gui. Partially based on the patch by Bert Wesarg [0]. > > The commits can be found in the topic branch 'py/revert-hunks-lines' > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > Once reviewed, pull the commits from > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > apply them locally, whichever you prefer. Let's see how we can work together by you playing the role of git-gui maintainer and the others on the list (including me) playing the role of reviewer and contributor. So I may keep an eye on the discussion on this thread, I may even comment on them myself, but you'll be the one waiting for the discussion to settle, adjusting the patches in response to reviews, etc. and making the final decision when/if the topic is done, at which time you'd be telling me to pull from you. > Pratyush Yadav (4): > git-gui: Move revert confirmation dialog creation to separate function > git-gui: Add option to disable the revert confirmation prompt > git-gui: Add the ability to revert selected lines > git-gui: Add the ability to revert selected hunk "Move" and "Add" after "git-gui:" would better be downcased to be more in line with the others in "git shortlog --no-merges"; I also think "allow doing X" is shorter and better way to say "add the ability to do X". If I am reading the first patch correctly, we already ask for confirmation before reverting local changes, and the steps 3 and 4 are about allowing partial reversion in addition to the wholesale reversion, right? An earlier objection from j6t sounded like we require users to respond to an extra dialog after this series, but that does not look like the case. Instead, step 2 adds a new feature to allow those to opt-out of the existing dialog (which may be reused to squelch the dialog to protect features added in steps 3 and 4). Am I reading the series correctly? Thanks. > > git-gui.sh | 40 +++++++++++++++++++++++++++++-- > lib/diff.tcl | 65 ++++++++++++++++++++++++++++++++++++++++++-------- > lib/index.tcl | 31 ++++++++++++++++-------- > lib/option.tcl | 1 + > 4 files changed, 115 insertions(+), 22 deletions(-) > > -- > 2.21.0 ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-22 22:34 ` [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines Junio C Hamano @ 2019-08-22 22:51 ` Pratyush Yadav 2019-08-23 6:04 ` Bert Wesarg 0 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-22 22:51 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Johannes Sixt, Bert Wesarg On 22/08/19 03:34PM, Junio C Hamano wrote: > Pratyush Yadav <me@yadavpratyush.com> writes: > > > This series adds the ability to revert selected lines and hunks in > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > The commits can be found in the topic branch 'py/revert-hunks-lines' > > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > > > Once reviewed, pull the commits from > > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > > apply them locally, whichever you prefer. > > Let's see how we can work together by you playing the role of > git-gui maintainer and the others on the list (including me) playing > the role of reviewer and contributor. So I may keep an eye on the > discussion on this thread, I may even comment on them myself, but > you'll be the one waiting for the discussion to settle, adjusting > the patches in response to reviews, etc. and making the final > decision when/if the topic is done, at which time you'd be telling > me to pull from you. > > > Pratyush Yadav (4): > > git-gui: Move revert confirmation dialog creation to separate function > > git-gui: Add option to disable the revert confirmation prompt > > git-gui: Add the ability to revert selected lines > > git-gui: Add the ability to revert selected hunk > > "Move" and "Add" after "git-gui:" would better be downcased to be > more in line with the others in "git shortlog --no-merges"; I also > think "allow doing X" is shorter and better way to say "add the > ability to do X". Will fix. Thanks. > If I am reading the first patch correctly, we already ask for > confirmation before reverting local changes, and the steps 3 and 4 > are about allowing partial reversion in addition to the wholesale > reversion, right? Yes. The ability to revert whole files already exists. This revert always asks for confirmation. Steps 3 and 4 allow for partial reverts. Step 2 allows the user to disable the confirmation dialog for both whole-file reverts and for partial reverts. > An earlier objection from j6t sounded like we > require users to respond to an extra dialog after this series, but > that does not look like the case. Instead, step 2 adds a new > feature to allow those to opt-out of the existing dialog (which may > be reused to squelch the dialog to protect features added in steps 3 > and 4). Am I reading the series correctly? Yes. The users always responded to an extra dialog for whole file reverts even before these changes. j6t was running a fork of git-gui which had the ability for partial reverts, and his fork did not ask for confirmation for partial reverts. Always asking for confirmation disrupts his workflow, so I added an option to disable it. It disables the dialog for partial reverts (which j6t cares about), and also for whole file reverts, which I added to maintain consistency. > > Thanks. > > > > > git-gui.sh | 40 +++++++++++++++++++++++++++++-- > > lib/diff.tcl | 65 ++++++++++++++++++++++++++++++++++++++++++-------- > > lib/index.tcl | 31 ++++++++++++++++-------- > > lib/option.tcl | 1 + > > 4 files changed, 115 insertions(+), 22 deletions(-) > > > > -- > > 2.21.0 -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-22 22:51 ` Pratyush Yadav @ 2019-08-23 6:04 ` Bert Wesarg 2019-08-23 16:04 ` Junio C Hamano 2019-08-23 16:44 ` Pratyush Yadav 0 siblings, 2 replies; 49+ messages in thread From: Bert Wesarg @ 2019-08-23 6:04 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Junio C Hamano, Git Mailing List, Johannes Sixt On Fri, Aug 23, 2019 at 12:51 AM Pratyush Yadav <me@yadavpratyush.com> wrote: > > On 22/08/19 03:34PM, Junio C Hamano wrote: > > Pratyush Yadav <me@yadavpratyush.com> writes: > > > > > This series adds the ability to revert selected lines and hunks in > > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > > > The commits can be found in the topic branch 'py/revert-hunks-lines' > > > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > > > > > Once reviewed, pull the commits from > > > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > > > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > > > apply them locally, whichever you prefer. > > > > Let's see how we can work together by you playing the role of > > git-gui maintainer and the others on the list (including me) playing > > the role of reviewer and contributor. So I may keep an eye on the > > discussion on this thread, I may even comment on them myself, but > > you'll be the one waiting for the discussion to settle, adjusting > > the patches in response to reviews, etc. and making the final > > decision when/if the topic is done, at which time you'd be telling > > me to pull from you. > > > > > Pratyush Yadav (4): > > > git-gui: Move revert confirmation dialog creation to separate function > > > git-gui: Add option to disable the revert confirmation prompt > > > git-gui: Add the ability to revert selected lines > > > git-gui: Add the ability to revert selected hunk > > > > "Move" and "Add" after "git-gui:" would better be downcased to be > > more in line with the others in "git shortlog --no-merges"; I also > > think "allow doing X" is shorter and better way to say "add the > > ability to do X". > > Will fix. Thanks. > > > If I am reading the first patch correctly, we already ask for > > confirmation before reverting local changes, and the steps 3 and 4 > > are about allowing partial reversion in addition to the wholesale > > reversion, right? > > Yes. The ability to revert whole files already exists. This revert > always asks for confirmation. Steps 3 and 4 allow for partial reverts. > Step 2 allows the user to disable the confirmation dialog for both > whole-file reverts and for partial reverts. > > > An earlier objection from j6t sounded like we > > require users to respond to an extra dialog after this series, but > > that does not look like the case. Instead, step 2 adds a new > > feature to allow those to opt-out of the existing dialog (which may > > be reused to squelch the dialog to protect features added in steps 3 > > and 4). Am I reading the series correctly? > > Yes. The users always responded to an extra dialog for whole file > reverts even before these changes. j6t was running a fork of git-gui > which had the ability for partial reverts, and his fork did not ask for > confirmation for partial reverts. Always asking for confirmation > disrupts his workflow, so I added an option to disable it. It disables > the dialog for partial reverts (which j6t cares about), and also for > whole file reverts, which I added to maintain consistency. as I'm the one who use this feature for more than 7 years, I can only object to this. I'm happy to have the confirmation dialog for the whole-file revert (the current state) but having the dialog also for partial revert would be too disruptive. And disabling both at the same time would not be an option for me. The thing is, that the partial revert "just don't happen by accident". Here are the minimum user actions needed to get to this dialog: 1. whole-file revert - do a Ctrl+J, more or less anywhere in the GUI 2. hunk revert/revert one unselected line - right click anywhere in the diff pane (thats around 60% of the window area) - move the mouse pointer down 3/4 menu items - click this menu item 3. partially revert selected lines - select some content in the diff pane by starting by pressing and holding a left click - end the selection by releasing the left click - move the mouse pointer down 3/4 menu items - click this menu item Thats always at least 2 user actions more than the whole-file revert. Thus this cannot happen by accident quite easily in comparison to the whole-file revert. And thats the reason why this dialog exists, from my point of view. I can see the need to disable the dialog for the whole-file revert, and IIRC that was also requested a long time ago on this list. But I don't see a reason to have this dialog also for the partial reverts as a safety measure. Best, Bert > > -- > Regards, > Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-23 6:04 ` Bert Wesarg @ 2019-08-23 16:04 ` Junio C Hamano 2019-08-23 16:44 ` Pratyush Yadav 1 sibling, 0 replies; 49+ messages in thread From: Junio C Hamano @ 2019-08-23 16:04 UTC (permalink / raw) To: Bert Wesarg; +Cc: Pratyush Yadav, Git Mailing List, Johannes Sixt Bert Wesarg <bert.wesarg@googlemail.com> writes: > The thing is, that the partial revert "just don't happen by accident". > Here are the minimum user actions needed to get to this dialog: > > 1. whole-file revert > > - do a Ctrl+J, more or less anywhere in the GUI > > 2. hunk revert/revert one unselected line > > - right click anywhere in the diff pane (thats around 60% of the window area) > - move the mouse pointer down 3/4 menu items > - click this menu item > > 3. partially revert selected lines > > - select some content in the diff pane by starting by pressing and > holding a left click > - end the selection by releasing the left click > - move the mouse pointer down 3/4 menu items > - click this menu item > > Thats always at least 2 user actions more than the whole-file revert. > Thus this cannot happen by accident quite easily in comparison to the > whole-file revert. And thats the reason why this dialog exists, from > my point of view. > > I can see the need to disable the dialog for the whole-file revert, > and IIRC that was also requested a long time ago on this list. But I > don't see a reason to have this dialog also for the partial reverts as > a safety measure. Thanks for walking us readers through your thought process. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-23 6:04 ` Bert Wesarg 2019-08-23 16:04 ` Junio C Hamano @ 2019-08-23 16:44 ` Pratyush Yadav [not found] ` <CAKPyHN0QbCDzcG8=zPP4-WHKqMk3R0sJ0BjXDR=zak3OfEa2bg@mail.gmail.com> 1 sibling, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-23 16:44 UTC (permalink / raw) To: Bert Wesarg; +Cc: Junio C Hamano, Git Mailing List, Johannes Sixt On 23/08/19 08:04AM, Bert Wesarg wrote: > On Fri, Aug 23, 2019 at 12:51 AM Pratyush Yadav <me@yadavpratyush.com> wrote: > > > > On 22/08/19 03:34PM, Junio C Hamano wrote: [...] > as I'm the one who use this feature for more than 7 years, I can only > object to this. I'm happy to have the confirmation dialog for the > whole-file revert (the current state) but having the dialog also for > partial revert would be too disruptive. And disabling both at the same > time would not be an option for me. > > The thing is, that the partial revert "just don't happen by accident". > Here are the minimum user actions needed to get to this dialog: > > 1. whole-file revert > > - do a Ctrl+J, more or less anywhere in the GUI Hmm, have to agree with you on this. It is kind of easy to trigger. But read below why I think partial reverts are just as easy to trigger. > 2. hunk revert/revert one unselected line > > - right click anywhere in the diff pane (thats around 60% of the window area) > - move the mouse pointer down 3/4 menu items > - click this menu item But what if you wanted to click "Stage hunk", and instead click "Revert hunk" instead. This is what I mean by "accidentally". This is even more a risk in the current layout of the buttons, which are in the order: Stage Hunk Revert Hunk Stage Lines Revert Lines In this layout, if you wanted to click Stage, you might end up clicking Revert instead, losing part of your work. Even if we switch up the layout a bit, I feel like the potential of "mis-aiming" your mouse is there, but it can certainly be improved. > 3. partially revert selected lines > > - select some content in the diff pane by starting by pressing and > holding a left click > - end the selection by releasing the left click > - move the mouse pointer down 3/4 menu items > - click this menu item > > Thats always at least 2 user actions more than the whole-file revert. > Thus this cannot happen by accident quite easily in comparison to the > whole-file revert. And thats the reason why this dialog exists, from > my point of view. > > I can see the need to disable the dialog for the whole-file revert, > and IIRC that was also requested a long time ago on this list. But I > don't see a reason to have this dialog also for the partial reverts as > a safety measure. I do (not too strongly, but I do), as I explained why above. It shouldn't be too difficult to have separate knobs for whole-file and partial reverts, but they will add two config options. Not necessarily a bad thing, I just thought the people who wanted to disable partial reverts would also want to disable whole-file reverts. But I have another suggestion in mind. I'll reply to one of the other emails in this thread about it. Please read it there, I'd rather not type it twice. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
[parent not found: <CAKPyHN0QbCDzcG8=zPP4-WHKqMk3R0sJ0BjXDR=zak3OfEa2bg@mail.gmail.com>]
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines [not found] ` <CAKPyHN0QbCDzcG8=zPP4-WHKqMk3R0sJ0BjXDR=zak3OfEa2bg@mail.gmail.com> @ 2019-08-25 11:57 ` Pratyush Yadav 0 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-25 11:57 UTC (permalink / raw) To: Bert Wesarg; +Cc: Junio C Hamano, Git Mailing List, Johannes Sixt On 23/08/19 11:12PM, Bert Wesarg wrote: > On Fri, Aug 23, 2019, 18:44 Pratyush Yadav <me@yadavpratyush.com> wrote: > > > On 23/08/19 08:04AM, Bert Wesarg wrote: > > > On Fri, Aug 23, 2019 at 12:51 AM Pratyush Yadav <me@yadavpratyush.com> > > wrote: > > > > > > > > On 22/08/19 03:34PM, Junio C Hamano wrote: > > [...] > > > as I'm the one who use this feature for more than 7 years, I can only > > > object to this. I'm happy to have the confirmation dialog for the > > > whole-file revert (the current state) but having the dialog also for > > > partial revert would be too disruptive. And disabling both at the same > > > time would not be an option for me. > > > > > > The thing is, that the partial revert "just don't happen by accident". > > > Here are the minimum user actions needed to get to this dialog: > > > > > > 1. whole-file revert > > > > > > - do a Ctrl+J, more or less anywhere in the GUI > > > > Hmm, have to agree with you on this. It is kind of easy to trigger. But > > read below why I think partial reverts are just as easy to trigger. > > > > > 2. hunk revert/revert one unselected line > > > > > > - right click anywhere in the diff pane (thats around 60% of the window > > area) > > > - move the mouse pointer down 3/4 menu items > > > - click this menu item > > > > But what if you wanted to click "Stage hunk", and instead click "Revert > > hunk" instead. This is what I mean by "accidentally". > > > > This is even more a risk in the current layout of the buttons, which are > > in the order: > > > > Stage Hunk > > Revert Hunk > > Stage Lines > > Revert Lines > > > > but this is your current layout! my layout in the patch from 2012 was (and > still is in my git-gui): > > Stage Hunk > Stage Line > ---------- > Revert Hunk > Revert Line > > > thats a separator inbetween! Looking at how this discussion has been going, I see that people really don't like the dialog, even with a config option. So I will send a re-roll with this layout of buttons and ability to undo last revert. I hope that will satisfy most people. > bert > > > > > In this layout, if you wanted to click Stage, you might end up clicking > > Revert instead, losing part of your work. Even if we switch up the > > layout a bit, I feel like the potential of "mis-aiming" your mouse is > > there, but it can certainly be improved. > > > > > 3. partially revert selected lines > > > > > > - select some content in the diff pane by starting by pressing and > > > holding a left click > > > - end the selection by releasing the left click > > > - move the mouse pointer down 3/4 menu items > > > - click this menu item > > > > > > Thats always at least 2 user actions more than the whole-file revert. > > > Thus this cannot happen by accident quite easily in comparison to the > > > whole-file revert. And thats the reason why this dialog exists, from > > > my point of view. > > > > > > I can see the need to disable the dialog for the whole-file revert, > > > and IIRC that was also requested a long time ago on this list. But I > > > don't see a reason to have this dialog also for the partial reverts as > > > a safety measure. > > > > I do (not too strongly, but I do), as I explained why above. > > > > It shouldn't be too difficult to have separate knobs for whole-file and > > partial reverts, but they will add two config options. Not necessarily a > > bad thing, I just thought the people who wanted to disable partial > > reverts would also want to disable whole-file reverts. > > > > But I have another suggestion in mind. I'll reply to one of the other > > emails in this thread about it. Please read it there, I'd rather not > > type it twice. > > > > -- > > Regards, > > Pratyush Yadav > > -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav ` (4 preceding siblings ...) 2019-08-22 22:34 ` [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines Junio C Hamano @ 2019-08-23 23:43 ` David Aguilar 2019-08-24 6:54 ` Bert Wesarg 2019-08-24 6:57 ` Bert Wesarg 5 siblings, 2 replies; 49+ messages in thread From: David Aguilar @ 2019-08-23 23:43 UTC (permalink / raw) To: Pratyush Yadav; +Cc: git, Junio C Hamano, Johannes Sixt, Bert Wesarg On Fri, Aug 23, 2019 at 03:31:03AM +0530, Pratyush Yadav wrote: > Hi, > > This series adds the ability to revert selected lines and hunks in > git-gui. Partially based on the patch by Bert Wesarg [0]. > > The commits can be found in the topic branch 'py/revert-hunks-lines' > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > Once reviewed, pull the commits from > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > apply them locally, whichever you prefer. > > Changes in v2: > - Add an option to disable the revert confirmation prompt as suggested > by Johannes Sixt. > - Base the patches on Pat's git-gui tree instead of git.git. We've had these features for years in git-cola. Please copy our keyboard shortcuts. IMO we should not re-invent the user interactions. http://git-cola.github.io/share/doc/git-cola/hotkeys.html Ctrl-u is our revert-unstaged-edits hotkeys. "s" is for staging/unstaging (or Ctrl-s if the file list is focused). The same hotkey is used for operating at the line level. If no lines are selected, the hunk surrounding the current cursor position is used. Please make keyboard interaction a first-class design consideration. I have a very strong opinion about the confirmation dialog, so I'll just mention that here since Hannes is on this thread. In cola we do have a confirmation dialog, and I strongly believe this is the correct behavior because it's an operation that drops data that cannot be recovered. In the other thread, it was mentioned that this dialog would be a nuisance. Perhaps that is true -- for the dialog that may have been implemented in this series (I haven't run it to verify). Let's dive into that concern. In git-cola we have a confirmation dialog and it is by no way a detriment to the workflow, and I use that feature all the time. Why? The reason is that we focused on the keyboard interaction. The workflow is as follows: Ctrl-u to initiate the revert action The prompt appears immediately. - Hitting any of "enter", "y", or "spacebar" will confirm the confirmation, and proceed. - Hitting any of "escape" or "n" will cancel the action. So essentially the workflow for the power user becomes "ctrl-u, enter" and that is such a tiny overhead that it really is not a bother at all. On the other hand, if I had to actually move my hand over to a mouse or trackpad and actually "click" on something then I would be super annoyed. That would be simply horrible with RSI in mind. OTOH having to hit "enter" or "spacebar" (which is the largest key on your keyboard, and your thumbs have good hefty muscles) is totally acceptable in my book because it strikes the right balance between safety for a destructive operation and convenience. Now, let's consider the alternative -- adding an option to disable the prompt. I don't like that. Why? It's yet another option. It's yet another thing to document, yet another code path, and yet another pitfall for a user who might run git-gui in a different configuration (and becomes surprised when revert doesn't prompt and suddenly loses their work). Do we really need an option, or do we need better usability instead? My opinion is that the latter is the real need. That's my $.02 from having used this feature in practice since 2013. -- David ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-23 23:43 ` David Aguilar @ 2019-08-24 6:54 ` Bert Wesarg 2019-08-24 6:57 ` Bert Wesarg 1 sibling, 0 replies; 49+ messages in thread From: Bert Wesarg @ 2019-08-24 6:54 UTC (permalink / raw) To: David Aguilar Cc: Pratyush Yadav, Git Mailing List, Junio C Hamano, Johannes Sixt On Sat, Aug 24, 2019 at 1:43 AM David Aguilar <davvid@gmail.com> wrote: > > On Fri, Aug 23, 2019 at 03:31:03AM +0530, Pratyush Yadav wrote: > > Hi, > > > > This series adds the ability to revert selected lines and hunks in > > git-gui. Partially based on the patch by Bert Wesarg [0]. > > > > The commits can be found in the topic branch 'py/revert-hunks-lines' > > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > > > Once reviewed, pull the commits from > > 415ce3f8582769d1d454b3796dc6c9c847cefa87 till > > 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and > > apply them locally, whichever you prefer. > > > > Changes in v2: > > - Add an option to disable the revert confirmation prompt as suggested > > by Johannes Sixt. > > - Base the patches on Pat's git-gui tree instead of git.git. > > > We've had these features for years in git-cola. this series does not introduce new hotkeys. > > Please copy our keyboard shortcuts. > IMO we should not re-invent the user interactions. > > http://git-cola.github.io/share/doc/git-cola/hotkeys.html my part for the homework: > > Ctrl-u is our revert-unstaged-edits hotkeys. https://github.com/patthoyts/git-gui/commit/b677c66e299c8754a6093cbd19ce71b0ad2a8a17 > "s" is for > staging/unstaging (or Ctrl-s if the file list is focused). https://github.com/patthoyts/git-gui/commit/cd16a6c9298778265a044e5f9a39b006277b32f2 https://github.com/patthoyts/git-gui/commit/e210e67451f22f97c1476d6b78b6fa7fdd5817f9#diff-ceba4b88c7e634c5401a4487d45d3ab4R774 Bert > > The same hotkey is used for operating at the line level. > If no lines are selected, the hunk surrounding the current cursor > position is used. > > Please make keyboard interaction a first-class design consideration. > ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-23 23:43 ` David Aguilar 2019-08-24 6:54 ` Bert Wesarg @ 2019-08-24 6:57 ` Bert Wesarg 2019-08-24 7:26 ` David Aguilar 2019-08-24 8:09 ` Johannes Sixt 1 sibling, 2 replies; 49+ messages in thread From: Bert Wesarg @ 2019-08-24 6:57 UTC (permalink / raw) To: David Aguilar Cc: Pratyush Yadav, Git Mailing List, Junio C Hamano, Johannes Sixt On Sat, Aug 24, 2019 at 1:43 AM David Aguilar <davvid@gmail.com> wrote: > > I have a very strong opinion about the confirmation dialog, so I'll just > mention that here since Hannes is on this thread. > > In cola we do have a confirmation dialog, and I strongly believe this is > the correct behavior because it's an operation that drops data that > cannot be recovered. > > In the other thread, it was mentioned that this dialog would be a > nuisance. Perhaps that is true -- for the dialog that may have been > implemented in this series (I haven't run it to verify). > > Let's dive into that concern. > > In git-cola we have a confirmation dialog and it is by no way a > detriment to the workflow, and I use that feature all the time. > Why? The reason is that we focused on the keyboard interaction. > > The workflow is as follows: > > Ctrl-u to initiate the revert action > The prompt appears immediately. > - Hitting any of "enter", "y", or "spacebar" will > confirm the confirmation, and proceed. > - Hitting any of "escape" or "n" will cancel the action. > > So essentially the workflow for the power user becomes "ctrl-u, enter" > and that is such a tiny overhead that it really is not a bother at all. > > On the other hand, if I had to actually move my hand over to a mouse or > trackpad and actually "click" on something then I would be super > annoyed. That would be simply horrible with RSI in mind. > I take this as a point for*not* having a confirmation dialog when doing the action per mouse. Which matches exactly my original implementation. > OTOH having to hit "enter" or "spacebar" (which is the largest key on > your keyboard, and your thumbs have good hefty muscles) is totally > acceptable in my book because it strikes the right balance between > safety for a destructive operation and convenience. > > Now, let's consider the alternative -- adding an option to disable the > prompt. I don't like that. > > Why? It's yet another option. It's yet another thing to document, yet > another code path, and yet another pitfall for a user who might run > git-gui in a different configuration (and becomes surprised when revert > doesn't prompt and suddenly loses their work). > > Do we really need an option, or do we need better usability instead? > My opinion is that the latter is the real need. > > > That's my $.02 from having used this feature in practice since 2013. 2012 Best, Bert > -- > David ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-24 6:57 ` Bert Wesarg @ 2019-08-24 7:26 ` David Aguilar 2019-08-24 8:09 ` Johannes Sixt 1 sibling, 0 replies; 49+ messages in thread From: David Aguilar @ 2019-08-24 7:26 UTC (permalink / raw) To: Bert Wesarg Cc: Pratyush Yadav, Git Mailing List, Junio C Hamano, Johannes Sixt On Sat, Aug 24, 2019 at 08:57:22AM +0200, Bert Wesarg wrote: > On Sat, Aug 24, 2019 at 1:43 AM David Aguilar <davvid@gmail.com> wrote: > > On the other hand, if I had to actually move my hand over to a mouse or > > trackpad and actually "click" on something then I would be super > > annoyed. That would be simply horrible with RSI in mind. > > > > I take this as a point for *not* having a confirmation dialog when > doing the action per mouse. Which matches exactly my original > implementation. +1 > > That's my $.02 from having used this feature in practice since 2013. > > 2012 Nice! ;-) I agree with everything you've written here. cheers, -- David ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-24 6:57 ` Bert Wesarg 2019-08-24 7:26 ` David Aguilar @ 2019-08-24 8:09 ` Johannes Sixt 1 sibling, 0 replies; 49+ messages in thread From: Johannes Sixt @ 2019-08-24 8:09 UTC (permalink / raw) To: Bert Wesarg Cc: David Aguilar, Pratyush Yadav, Git Mailing List, Junio C Hamano Am 24.08.19 um 08:57 schrieb Bert Wesarg: > On Sat, Aug 24, 2019 at 1:43 AM David Aguilar <davvid@gmail.com> wrote: >> On the other hand, if I had to actually move my hand over to a mouse or >> trackpad and actually "click" on something then I would be super >> annoyed. That would be simply horrible with RSI in mind. >> > > I take this as a point for*not* having a confirmation dialog when > doing the action per mouse. Which matches exactly my original > implementation. I totally agree. -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav ` (5 preceding siblings ...) 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav @ 2019-08-28 21:57 ` Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 1/4] git-gui: allow reverting selected lines Pratyush Yadav ` (4 more replies) 6 siblings, 5 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-28 21:57 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Hi, This series adds the ability to revert selected lines and hunks in git-gui. Partially based on the patch by Bert Wesarg [0]. The commits can be found in the topic branch 'py/revert-hunks-lines' at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines Changes in v3: - Drop the confirmation dialog on partial reverts. It is still there for full file reverts (which was the original behaviour). - Allow undoing the last revert. - Update the context menu button layout. In v2, the layout was: Stage Hunk Revert Hunk Stage Lines Revert Lines Now it is: Stage Hunk Stage Lines ----------- Revert Hunk Revert Lines Undo Last Revert - Return early when applying a patch fails. This is useful for this series because in that case we don't save a faulty patch in last_revert, causing the same error to pop up when reverting the patch that failed to apply in the first place. Changes in v2: - Add an option to disable the revert confirmation prompt as suggested by Johannes Sixt. - Base the patches on Pat's git-gui tree instead of git.git. [0] https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wesarg@googlemail.com/ Pratyush Yadav (4): git-gui: allow reverting selected lines git-gui: allow reverting selected hunk git-gui: return early when patch fails to apply git-gui: allow undoing last revert git-gui.sh | 57 +++++++++++++++++++++++++++++-- lib/diff.tcl | 96 ++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 135 insertions(+), 18 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v3 1/4] git-gui: allow reverting selected lines 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav @ 2019-08-28 21:57 ` Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 2/4] git-gui: allow reverting selected hunk Pratyush Yadav ` (3 subsequent siblings) 4 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-28 21:57 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Just like the user can select lines to stage or unstage, add the ability to revert selected lines. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui.sh | 25 ++++++++++++++++++++++++- lib/diff.tcl | 20 ++++++++++++++------ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 5bc21b8..6d4d002 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -3587,10 +3587,16 @@ set ui_diff_applyhunk [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] $ctxm add command \ -label [mc "Apply/Reverse Line"] \ - -command {apply_range_or_line $cursorX $cursorY; do_rescan} + -command {apply_or_revert_range_or_line $cursorX $cursorY 0; do_rescan} set ui_diff_applyline [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state] $ctxm add separator +$ctxm add command \ + -label [mc "Revert Line"] \ + -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} +set ui_diff_revertline [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] +$ctxm add separator $ctxm add command \ -label [mc "Show Less Context"] \ -command show_less_context @@ -3687,15 +3693,19 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { set l [mc "Unstage Hunk From Commit"] if {$has_range} { set t [mc "Unstage Lines From Commit"] + set r [mc "Revert Lines"] } else { set t [mc "Unstage Line From Commit"] + set r [mc "Revert Line"] } } else { set l [mc "Stage Hunk For Commit"] if {$has_range} { set t [mc "Stage Lines For Commit"] + set r [mc "Revert Lines"] } else { set t [mc "Stage Line For Commit"] + set r [mc "Revert Line"] } } if {$::is_3way_diff @@ -3706,11 +3716,24 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { || [string match {T?} $state] || [has_textconv $current_diff_path]} { set s disabled + set revert_state disabled } else { set s normal + + # Only allow reverting changes in the working tree. If + # the user wants to revert changes in the index, they + # need to unstage those first. + if {$::ui_workdir eq $::current_diff_side} { + set revert_state normal + } else { + set revert_state disabled + } } + $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l $ctxm entryconf $::ui_diff_applyline -state $s -label $t + $ctxm entryconf $::ui_diff_revertline -state $revert_state \ + -label $r tk_popup $ctxm $X $Y } } diff --git a/lib/diff.tcl b/lib/diff.tcl index 4cae10a..d6bee29 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -55,7 +55,7 @@ proc reshow_diff {{after {}}} { proc force_diff_encoding {enc} { global current_diff_path - + if {$current_diff_path ne {}} { force_path_encoding $current_diff_path $enc reshow_diff @@ -640,7 +640,7 @@ proc apply_hunk {x y} { } } -proc apply_range_or_line {x y} { +proc apply_or_revert_range_or_line {x y revert} { global current_diff_path current_diff_header current_diff_side global ui_diff ui_index file_states @@ -660,19 +660,27 @@ proc apply_range_or_line {x y} { if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return - set apply_cmd {apply --cached --whitespace=nowarn} + set apply_cmd {apply --whitespace=nowarn} set mi [lindex $file_states($current_diff_path) 0] if {$current_diff_side eq $ui_index} { set failed_msg [mc "Failed to unstage selected line."] set to_context {+} - lappend apply_cmd --reverse + lappend apply_cmd --reverse --cached if {[string index $mi 0] ne {M}} { unlock_index return } } else { - set failed_msg [mc "Failed to stage selected line."] - set to_context {-} + if {$revert} { + set failed_msg [mc "Failed to revert selected line."] + set to_context {+} + lappend apply_cmd --reverse + } else { + set failed_msg [mc "Failed to stage selected line."] + set to_context {-} + lappend apply_cmd --cached + } + if {[string index $mi 1] ne {M}} { unlock_index return -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v3 2/4] git-gui: allow reverting selected hunk 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 1/4] git-gui: allow reverting selected lines Pratyush Yadav @ 2019-08-28 21:57 ` Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 3/4] git-gui: return early when patch fails to apply Pratyush Yadav ` (2 subsequent siblings) 4 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-28 21:57 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Just like the user can select a hunk to stage or unstage, add the ability to revert hunks. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui.sh | 14 +++++++++++++- lib/diff.tcl | 21 ++++++++++++++++----- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 6d4d002..1592544 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -3582,7 +3582,7 @@ set ctxm .vpane.lower.diff.body.ctxm menu $ctxm -tearoff 0 $ctxm add command \ -label [mc "Apply/Reverse Hunk"] \ - -command {apply_hunk $cursorX $cursorY} + -command {apply_or_revert_hunk $cursorX $cursorY 0} set ui_diff_applyhunk [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state] $ctxm add command \ @@ -3591,6 +3591,11 @@ $ctxm add command \ set ui_diff_applyline [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state] $ctxm add separator +$ctxm add command \ + -label [mc "Revert Hunk"] \ + -command {apply_or_revert_hunk $cursorX $cursorY 1} +set ui_diff_reverthunk [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_reverthunk -state] $ctxm add command \ -label [mc "Revert Line"] \ -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} @@ -3691,6 +3696,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}] if {$::ui_index eq $::current_diff_side} { set l [mc "Unstage Hunk From Commit"] + set h [mc "Revert Hunk"] + if {$has_range} { set t [mc "Unstage Lines From Commit"] set r [mc "Revert Lines"] @@ -3700,6 +3707,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { } } else { set l [mc "Stage Hunk For Commit"] + set h [mc "Revert Hunk"] + if {$has_range} { set t [mc "Stage Lines For Commit"] set r [mc "Revert Lines"] @@ -3734,6 +3743,9 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { $ctxm entryconf $::ui_diff_applyline -state $s -label $t $ctxm entryconf $::ui_diff_revertline -state $revert_state \ -label $r + $ctxm entryconf $::ui_diff_reverthunk -state $revert_state \ + -label $h + tk_popup $ctxm $X $Y } } diff --git a/lib/diff.tcl b/lib/diff.tcl index d6bee29..ffca788 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -567,24 +567,31 @@ proc read_diff {fd conflict_size cont_info} { } } -proc apply_hunk {x y} { +proc apply_or_revert_hunk {x y revert} { global current_diff_path current_diff_header current_diff_side global ui_diff ui_index file_states if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return - set apply_cmd {apply --cached --whitespace=nowarn} + set apply_cmd {apply --whitespace=nowarn} set mi [lindex $file_states($current_diff_path) 0] if {$current_diff_side eq $ui_index} { set failed_msg [mc "Failed to unstage selected hunk."] - lappend apply_cmd --reverse + lappend apply_cmd --reverse --cached if {[string index $mi 0] ne {M}} { unlock_index return } } else { - set failed_msg [mc "Failed to stage selected hunk."] + if {$revert} { + set failed_msg [mc "Failed to revert selected hunk."] + lappend apply_cmd --reverse + } else { + set failed_msg [mc "Failed to stage selected hunk."] + lappend apply_cmd --cached + } + if {[string index $mi 1] ne {M}} { unlock_index return @@ -619,13 +626,17 @@ proc apply_hunk {x y} { $ui_diff delete $s_lno $e_lno $ui_diff conf -state disabled + # Check if the hunk was the last one in the file. if {[$ui_diff get 1.0 end] eq "\n"} { set o _ } else { set o ? } - if {$current_diff_side eq $ui_index} { + # Update the status flags. + if {$revert} { + set mi [string index $mi 0]$o + } elseif {$current_diff_side eq $ui_index} { set mi ${o}M } elseif {[string index $mi 0] eq {_}} { set mi M$o -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v3 3/4] git-gui: return early when patch fails to apply 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 1/4] git-gui: allow reverting selected lines Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 2/4] git-gui: allow reverting selected hunk Pratyush Yadav @ 2019-08-28 21:57 ` Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 4/4] git-gui: allow undoing last revert Pratyush Yadav 2019-09-10 19:21 ` [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav 4 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-08-28 21:57 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg In the procedure apply_or_revert_range_or_line, if the patch does not apply successfully, a dialog is shown, but execution proceeds after that. Instead, return early on error so the parts that come after this don't work on top of an error state. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- lib/diff.tcl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/diff.tcl b/lib/diff.tcl index ffca788..0659029 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -848,6 +848,8 @@ proc apply_or_revert_range_or_line {x y revert} { puts -nonewline $p $wholepatch close $p} err]} { error_popup "$failed_msg\n\n$err" + unlock_index + return } unlock_index -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v3 4/4] git-gui: allow undoing last revert 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav ` (2 preceding siblings ...) 2019-08-28 21:57 ` [PATCH v3 3/4] git-gui: return early when patch fails to apply Pratyush Yadav @ 2019-08-28 21:57 ` Pratyush Yadav 2019-10-21 9:16 ` Bert Wesarg 2019-09-10 19:21 ` [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav 4 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-08-28 21:57 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Junio C Hamano, Johannes Sixt, Bert Wesarg Accidental clicks on the revert hunk/lines buttons can cause loss of work, and can be frustrating. So, allow undoing the last revert. Right now, a stack or deque are not being used for the sake of simplicity, so only one undo is possible. Any reverts before the previous one are lost. Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> --- git-gui.sh | 18 +++++++++++++++++- lib/diff.tcl | 53 ++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 1592544..e03a2d2 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -1350,6 +1350,8 @@ set is_submodule_diff 0 set is_conflict_diff 0 set selected_commit_type new set diff_empty_count 0 +set last_revert {} +set last_revert_enc {} set nullid "0000000000000000000000000000000000000000" set nullid2 "0000000000000000000000000000000000000001" @@ -3601,6 +3603,11 @@ $ctxm add command \ -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} set ui_diff_revertline [$ctxm index last] lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] +$ctxm add command \ + -label [mc "Undo Last Revert"] \ + -command {undo_last_revert; do_rescan} +set ui_diff_undorevert [$ctxm index last] +lappend diff_actions [list $ctxm entryconf $ui_diff_undorevert -state] $ctxm add separator $ctxm add command \ -label [mc "Show Less Context"] \ @@ -3680,7 +3687,7 @@ proc has_textconv {path} { } proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { - global current_diff_path file_states + global current_diff_path file_states last_revert set ::cursorX $x set ::cursorY $y if {[info exists file_states($current_diff_path)]} { @@ -3694,6 +3701,7 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { tk_popup $ctxmsm $X $Y } else { set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}] + set u [mc "Undo Last Revert"] if {$::ui_index eq $::current_diff_side} { set l [mc "Unstage Hunk From Commit"] set h [mc "Revert Hunk"] @@ -3739,12 +3747,20 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { } } + if {$last_revert eq {}} { + set undo_state disabled + } else { + set undo_state normal + } + $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l $ctxm entryconf $::ui_diff_applyline -state $s -label $t $ctxm entryconf $::ui_diff_revertline -state $revert_state \ -label $r $ctxm entryconf $::ui_diff_reverthunk -state $revert_state \ -label $h + $ctxm entryconf $::ui_diff_undorevert -state $undo_state \ + -label $u tk_popup $ctxm $X $Y } diff --git a/lib/diff.tcl b/lib/diff.tcl index 0659029..96288fc 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -569,7 +569,7 @@ proc read_diff {fd conflict_size cont_info} { proc apply_or_revert_hunk {x y revert} { global current_diff_path current_diff_header current_diff_side - global ui_diff ui_index file_states + global ui_diff ui_index file_states last_revert last_revert_enc if {$current_diff_path eq {} || $current_diff_header eq {}} return if {![lock_index apply_hunk]} return @@ -610,18 +610,25 @@ proc apply_or_revert_hunk {x y revert} { set e_lno end } + set wholepatch "$current_diff_header[$ui_diff get $s_lno $e_lno]" + if {[catch { set enc [get_path_encoding $current_diff_path] set p [eval git_write $apply_cmd] fconfigure $p -translation binary -encoding $enc - puts -nonewline $p $current_diff_header - puts -nonewline $p [$ui_diff get $s_lno $e_lno] + puts -nonewline $p $wholepatch close $p} err]} { error_popup "$failed_msg\n\n$err" unlock_index return } + if {$revert} { + # Save a copy of this patch for undoing reverts. + set last_revert $wholepatch + set last_revert_enc $enc + } + $ui_diff conf -state normal $ui_diff delete $s_lno $e_lno $ui_diff conf -state disabled @@ -653,7 +660,7 @@ proc apply_or_revert_hunk {x y revert} { proc apply_or_revert_range_or_line {x y revert} { global current_diff_path current_diff_header current_diff_side - global ui_diff ui_index file_states + global ui_diff ui_index file_states last_revert set selected [$ui_diff tag nextrange sel 0.0] @@ -852,5 +859,43 @@ proc apply_or_revert_range_or_line {x y revert} { return } + if {$revert} { + # Save a copy of this patch for undoing reverts. + set last_revert $current_diff_header$wholepatch + set last_revert_enc $enc + } + + unlock_index +} + +# Undo the last line/hunk reverted. When hunks and lines are reverted, a copy +# of the diff applied is saved. Re-apply that diff to undo the revert. +# +# Right now, we only use a single variable to hold the copy, and not a +# stack/deque for simplicity, so multiple undos are not possible. Maybe this +# can be added if the need for something like this is felt in the future. +proc undo_last_revert {} { + global last_revert current_diff_path current_diff_header + global last_revert_enc + + if {$last_revert eq {}} return + if {![lock_index apply_hunk]} return + + set apply_cmd {apply --whitespace=nowarn} + set failed_msg [mc "Failed to undo last revert."] + + if {[catch { + set enc $last_revert_enc + set p [eval git_write $apply_cmd] + fconfigure $p -translation binary -encoding $enc + puts -nonewline $p $last_revert + close $p} err]} { + error_popup "$failed_msg\n\n$err" + unlock_index + return + } + + set last_revert {} + unlock_index } -- 2.21.0 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-08-28 21:57 ` [PATCH v3 4/4] git-gui: allow undoing last revert Pratyush Yadav @ 2019-10-21 9:16 ` Bert Wesarg 2019-10-21 19:04 ` Pratyush Yadav 2019-10-21 19:35 ` Johannes Sixt 0 siblings, 2 replies; 49+ messages in thread From: Bert Wesarg @ 2019-10-21 9:16 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt Dear Pratyush, I just noticed that the 'Revert Last Hunk' menu entry is enabled in the stage-list. But I think it should be disabled, like the 'Revert Hunk' and 'Revert Line' menu entry. Can you confirm this? Thanks. Bert On Wed, Aug 28, 2019 at 11:57 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Accidental clicks on the revert hunk/lines buttons can cause loss of > work, and can be frustrating. So, allow undoing the last revert. > > Right now, a stack or deque are not being used for the sake of > simplicity, so only one undo is possible. Any reverts before the > previous one are lost. > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> > --- > git-gui.sh | 18 +++++++++++++++++- > lib/diff.tcl | 53 ++++++++++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 66 insertions(+), 5 deletions(-) > > diff --git a/git-gui.sh b/git-gui.sh > index 1592544..e03a2d2 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -1350,6 +1350,8 @@ set is_submodule_diff 0 > set is_conflict_diff 0 > set selected_commit_type new > set diff_empty_count 0 > +set last_revert {} > +set last_revert_enc {} > > set nullid "0000000000000000000000000000000000000000" > set nullid2 "0000000000000000000000000000000000000001" > @@ -3601,6 +3603,11 @@ $ctxm add command \ > -command {apply_or_revert_range_or_line $cursorX $cursorY 1; do_rescan} > set ui_diff_revertline [$ctxm index last] > lappend diff_actions [list $ctxm entryconf $ui_diff_revertline -state] > +$ctxm add command \ > + -label [mc "Undo Last Revert"] \ > + -command {undo_last_revert; do_rescan} > +set ui_diff_undorevert [$ctxm index last] > +lappend diff_actions [list $ctxm entryconf $ui_diff_undorevert -state] > $ctxm add separator > $ctxm add command \ > -label [mc "Show Less Context"] \ > @@ -3680,7 +3687,7 @@ proc has_textconv {path} { > } > > proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > - global current_diff_path file_states > + global current_diff_path file_states last_revert > set ::cursorX $x > set ::cursorY $y > if {[info exists file_states($current_diff_path)]} { > @@ -3694,6 +3701,7 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > tk_popup $ctxmsm $X $Y > } else { > set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}] > + set u [mc "Undo Last Revert"] > if {$::ui_index eq $::current_diff_side} { > set l [mc "Unstage Hunk From Commit"] > set h [mc "Revert Hunk"] > @@ -3739,12 +3747,20 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} { > } > } > > + if {$last_revert eq {}} { > + set undo_state disabled > + } else { > + set undo_state normal > + } > + > $ctxm entryconf $::ui_diff_applyhunk -state $s -label $l > $ctxm entryconf $::ui_diff_applyline -state $s -label $t > $ctxm entryconf $::ui_diff_revertline -state $revert_state \ > -label $r > $ctxm entryconf $::ui_diff_reverthunk -state $revert_state \ > -label $h > + $ctxm entryconf $::ui_diff_undorevert -state $undo_state \ > + -label $u > > tk_popup $ctxm $X $Y > } > diff --git a/lib/diff.tcl b/lib/diff.tcl > index 0659029..96288fc 100644 > --- a/lib/diff.tcl > +++ b/lib/diff.tcl > @@ -569,7 +569,7 @@ proc read_diff {fd conflict_size cont_info} { > > proc apply_or_revert_hunk {x y revert} { > global current_diff_path current_diff_header current_diff_side > - global ui_diff ui_index file_states > + global ui_diff ui_index file_states last_revert last_revert_enc > > if {$current_diff_path eq {} || $current_diff_header eq {}} return > if {![lock_index apply_hunk]} return > @@ -610,18 +610,25 @@ proc apply_or_revert_hunk {x y revert} { > set e_lno end > } > > + set wholepatch "$current_diff_header[$ui_diff get $s_lno $e_lno]" > + > if {[catch { > set enc [get_path_encoding $current_diff_path] > set p [eval git_write $apply_cmd] > fconfigure $p -translation binary -encoding $enc > - puts -nonewline $p $current_diff_header > - puts -nonewline $p [$ui_diff get $s_lno $e_lno] > + puts -nonewline $p $wholepatch > close $p} err]} { > error_popup "$failed_msg\n\n$err" > unlock_index > return > } > > + if {$revert} { > + # Save a copy of this patch for undoing reverts. > + set last_revert $wholepatch > + set last_revert_enc $enc > + } > + > $ui_diff conf -state normal > $ui_diff delete $s_lno $e_lno > $ui_diff conf -state disabled > @@ -653,7 +660,7 @@ proc apply_or_revert_hunk {x y revert} { > > proc apply_or_revert_range_or_line {x y revert} { > global current_diff_path current_diff_header current_diff_side > - global ui_diff ui_index file_states > + global ui_diff ui_index file_states last_revert > > set selected [$ui_diff tag nextrange sel 0.0] > > @@ -852,5 +859,43 @@ proc apply_or_revert_range_or_line {x y revert} { > return > } > > + if {$revert} { > + # Save a copy of this patch for undoing reverts. > + set last_revert $current_diff_header$wholepatch > + set last_revert_enc $enc > + } > + > + unlock_index > +} > + > +# Undo the last line/hunk reverted. When hunks and lines are reverted, a copy > +# of the diff applied is saved. Re-apply that diff to undo the revert. > +# > +# Right now, we only use a single variable to hold the copy, and not a > +# stack/deque for simplicity, so multiple undos are not possible. Maybe this > +# can be added if the need for something like this is felt in the future. > +proc undo_last_revert {} { > + global last_revert current_diff_path current_diff_header > + global last_revert_enc > + > + if {$last_revert eq {}} return > + if {![lock_index apply_hunk]} return > + > + set apply_cmd {apply --whitespace=nowarn} > + set failed_msg [mc "Failed to undo last revert."] > + > + if {[catch { > + set enc $last_revert_enc > + set p [eval git_write $apply_cmd] > + fconfigure $p -translation binary -encoding $enc > + puts -nonewline $p $last_revert > + close $p} err]} { > + error_popup "$failed_msg\n\n$err" > + unlock_index > + return > + } > + > + set last_revert {} > + > unlock_index > } > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-10-21 9:16 ` Bert Wesarg @ 2019-10-21 19:04 ` Pratyush Yadav 2019-10-22 8:17 ` Bert Wesarg 2019-10-21 19:35 ` Johannes Sixt 1 sibling, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-10-21 19:04 UTC (permalink / raw) To: Bert Wesarg; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt On 21/10/19 11:16AM, Bert Wesarg wrote: > Dear Pratyush, > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > the stage-list. But I think it should be disabled, like the 'Revert > Hunk' and 'Revert Line' menu entry. I'm not sure what you mean. There is no "Revert Last Hunk" menu entry (I assume you are talking about the context menu in the diff view that we open by right clicking). My guess is that you mean the "Undo Last Revert" option. And you want it disabled if the diff shown is of a staged file, correct? I'm not sure if disabling it would be a good idea. Say I revert a hunk or line while the file is not staged, and stage the rest of the file. This would mean that file is no longer in the "Unstaged Changes" widget. Now if I choose the file from the "Staged Changes" widget, I get the option to undo the last revert. If I hit that, it will put whatever I reverted in the "Unstaged Changes" widget. So, now part of the file that was reverted is in "Unstaged Changes", and the rest in "Unstaged Changes". IIUC, this is what you think should not happen, correct? What's wrong with allowing the user to undo reverts from anywhere? The way I see it, it doesn't really matter what file is selected or whether it is staged or not, the action of the undo remains the same, so it makes sense to me to allow it from anywhere. > Can you confirm this? > > On Wed, Aug 28, 2019 at 11:57 PM Pratyush Yadav <me@yadavpratyush.com> > wrote: > > > > Accidental clicks on the revert hunk/lines buttons can cause loss of > > work, and can be frustrating. So, allow undoing the last revert. > > > > Right now, a stack or deque are not being used for the sake of > > simplicity, so only one undo is possible. Any reverts before the > > previous one are lost. > > > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-10-21 19:04 ` Pratyush Yadav @ 2019-10-22 8:17 ` Bert Wesarg 2019-10-23 18:57 ` Pratyush Yadav 0 siblings, 1 reply; 49+ messages in thread From: Bert Wesarg @ 2019-10-22 8:17 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt On Mon, Oct 21, 2019 at 9:04 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > On 21/10/19 11:16AM, Bert Wesarg wrote: > > Dear Pratyush, > > > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > > the stage-list. But I think it should be disabled, like the 'Revert > > Hunk' and 'Revert Line' menu entry. > > I'm not sure what you mean. There is no "Revert Last Hunk" menu entry (I > assume you are talking about the context menu in the diff view that we > open by right clicking). > > My guess is that you mean the "Undo Last Revert" option. And you want it > disabled if the diff shown is of a staged file, correct? > > I'm not sure if disabling it would be a good idea. > > Say I revert a hunk or line while the file is not staged, and stage the > rest of the file. This would mean that file is no longer in the > "Unstaged Changes" widget. Now if I choose the file from the "Staged > Changes" widget, I get the option to undo the last revert. If I hit > that, it will put whatever I reverted in the "Unstaged Changes" widget. > So, now part of the file that was reverted is in "Unstaged Changes", and > the rest in "Unstaged Changes". > Sorry for this confusion. > IIUC, this is what you think should not happen, correct? What's wrong > with allowing the user to undo reverts from anywhere? The 'Undo' changes the worktree not the staged content,. > > The way I see it, it doesn't really matter what file is selected or > whether it is staged or not, the action of the undo remains the same, so > it makes sense to me to allow it from anywhere. It would make sense to undo the revert on the staged content, if there are no more changes to this file in the worktree. I.e., you wont be able to apply the 'undo' anymore to the worktree file if it is not listed anymore. Though even that case should be able to implement. Though the undo is currently not bound to a specific path. This may be the cause of our different view. I though the undo is bound to the path it was recorded, thus also would only be available when showing a diff on this path again. Therefore I think, having the 'Undo Last Revert' in the context menu may not be the best place to begin with, or at least indicate that this 'undo' operation does not necceseritly operate on the file currently shown. Bertt > > > Can you confirm this? > > > > On Wed, Aug 28, 2019 at 11:57 PM Pratyush Yadav <me@yadavpratyush.com> > > wrote: > > > > > > Accidental clicks on the revert hunk/lines buttons can cause loss of > > > work, and can be frustrating. So, allow undoing the last revert. > > > > > > Right now, a stack or deque are not being used for the sake of > > > simplicity, so only one undo is possible. Any reverts before the > > > previous one are lost. > > > > > > Signed-off-by: Pratyush Yadav <me@yadavpratyush.com> > > -- > Regards, > Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-10-22 8:17 ` Bert Wesarg @ 2019-10-23 18:57 ` Pratyush Yadav 0 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-10-23 18:57 UTC (permalink / raw) To: Bert Wesarg; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt On 22/10/19 10:17AM, Bert Wesarg wrote: > On Mon, Oct 21, 2019 at 9:04 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > > > On 21/10/19 11:16AM, Bert Wesarg wrote: > > > Dear Pratyush, > > > > > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > > > the stage-list. But I think it should be disabled, like the 'Revert > > > Hunk' and 'Revert Line' menu entry. > > > > I'm not sure what you mean. There is no "Revert Last Hunk" menu entry (I > > assume you are talking about the context menu in the diff view that we > > open by right clicking). > > > > My guess is that you mean the "Undo Last Revert" option. And you want it > > disabled if the diff shown is of a staged file, correct? > > > > I'm not sure if disabling it would be a good idea. > > > > Say I revert a hunk or line while the file is not staged, and stage the > > rest of the file. This would mean that file is no longer in the > > "Unstaged Changes" widget. Now if I choose the file from the "Staged > > Changes" widget, I get the option to undo the last revert. If I hit > > that, it will put whatever I reverted in the "Unstaged Changes" widget. > > So, now part of the file that was reverted is in "Unstaged Changes", and > > the rest in "Unstaged Changes". > > > > Sorry for this confusion. > > > IIUC, this is what you think should not happen, correct? What's wrong > > with allowing the user to undo reverts from anywhere? > > The 'Undo' changes the worktree not the staged content,. > > > > > The way I see it, it doesn't really matter what file is selected or > > whether it is staged or not, the action of the undo remains the same, so > > it makes sense to me to allow it from anywhere. > > It would make sense to undo the revert on the staged content, if there > are no more changes to this file in the worktree. I.e., you wont be > able to apply the 'undo' anymore to the worktree file if it is not > listed anymore. Though even that case should be able to implement. > Though the undo is currently not bound to a specific path. This may be I did it this way because I thought it would be best to go for the simplest implementation first, and then re-evaluate if needed. And nobody objected in the reviews. Maybe I didn't do a good job of clarifying the exact behaviour in the commit message. I'm not opposed to something like a per-path undo though. But I'm not sure if that will cause any confusion as to what is being undone. Or maybe the current version is more confusing. I can't really say since I am the one who implemented it, so I know exactly what it does. > the cause of our different view. I though the undo is bound to the > path it was recorded, thus also would only be available when showing a > diff on this path again. Therefore I think, having the 'Undo Last > Revert' in the context menu may not be the best place to begin with, > or at least indicate that this 'undo' operation does not necceseritly > operate on the file currently shown. Where else would you put it if not the context menu? There is the option of putting it in the menu bar at the top under the "Edit" menu entry, but I think that is too hidden. The original idea of the feature was that you could quickly undo an accidental revert. Hiding this in the menu bar would hurt discoverability, and some people might not realize they could have undone the revert. Is there a better place to put it that I'm missing? -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-10-21 9:16 ` Bert Wesarg 2019-10-21 19:04 ` Pratyush Yadav @ 2019-10-21 19:35 ` Johannes Sixt 2019-10-22 7:46 ` Bert Wesarg 1 sibling, 1 reply; 49+ messages in thread From: Johannes Sixt @ 2019-10-21 19:35 UTC (permalink / raw) To: Bert Wesarg; +Cc: Pratyush Yadav, Git Mailing List, Junio C Hamano Am 21.10.19 um 11:16 schrieb Bert Wesarg: > Dear Pratyush, > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > the stage-list. But I think it should be disabled, like the 'Revert > Hunk' and 'Revert Line' menu entry. > > Can you confirm this? Technically, it need not be disabled because the hunk being reverted does not depend on the contents of any of diffs that can be shown. The entry should be disabled if reverting the stored hunk fails. But to know that, it would have to be tried: the file could have been edited since the hunk was generated so that the reversal of the hunk fails. Not sure what to do, though. -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-10-21 19:35 ` Johannes Sixt @ 2019-10-22 7:46 ` Bert Wesarg 2019-10-23 19:00 ` Pratyush Yadav 0 siblings, 1 reply; 49+ messages in thread From: Bert Wesarg @ 2019-10-22 7:46 UTC (permalink / raw) To: Johannes Sixt; +Cc: Pratyush Yadav, Git Mailing List, Junio C Hamano On Mon, Oct 21, 2019 at 9:35 PM Johannes Sixt <j6t@kdbg.org> wrote: > > Am 21.10.19 um 11:16 schrieb Bert Wesarg: > > Dear Pratyush, > > > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > > the stage-list. But I think it should be disabled, like the 'Revert > > Hunk' and 'Revert Line' menu entry. > > > > Can you confirm this? > > Technically, it need not be disabled because the hunk being reverted > does not depend on the contents of any of diffs that can be shown. > > The entry should be disabled if reverting the stored hunk fails. But to > know that, it would have to be tried: the file could have been edited > since the hunk was generated so that the reversal of the hunk fails. But the "Undo" changes the worktree not the stage, sure it indirectly also changes the view of the staged content, but that is only a secondary effect. As I only can revert in the worktree list, I think we should be consistent and also only allow to undo the revert in the worktree list. And I think it is independent of 'does the undo apply at all' question. Bert > > Not sure what to do, though. > > -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 4/4] git-gui: allow undoing last revert 2019-10-22 7:46 ` Bert Wesarg @ 2019-10-23 19:00 ` Pratyush Yadav 0 siblings, 0 replies; 49+ messages in thread From: Pratyush Yadav @ 2019-10-23 19:00 UTC (permalink / raw) To: Bert Wesarg; +Cc: Johannes Sixt, Git Mailing List, Junio C Hamano On 22/10/19 09:46AM, Bert Wesarg wrote: > On Mon, Oct 21, 2019 at 9:35 PM Johannes Sixt <j6t@kdbg.org> wrote: > > > > Am 21.10.19 um 11:16 schrieb Bert Wesarg: > > > Dear Pratyush, > > > > > > I just noticed that the 'Revert Last Hunk' menu entry is enabled in > > > the stage-list. But I think it should be disabled, like the 'Revert > > > Hunk' and 'Revert Line' menu entry. > > > > > > Can you confirm this? > > > > Technically, it need not be disabled because the hunk being reverted > > does not depend on the contents of any of diffs that can be shown. > > > > The entry should be disabled if reverting the stored hunk fails. But to > > know that, it would have to be tried: the file could have been edited > > since the hunk was generated so that the reversal of the hunk fails. > > But the "Undo" changes the worktree not the stage, sure it indirectly > also changes the view of the staged content, but that is only a I don't think the "Undo Last Revert" should affect "staged content" in any way. In fact, if it does, it is probably a bug. A more detailed reply is to your other email. I just wanted to clarify that an undo _should not_ affect the staged content. > secondary effect. As I only can revert in the worktree list, I think > we should be consistent and also only allow to undo the revert in the > worktree list. > > And I think it is independent of 'does the undo apply at all' question. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav ` (3 preceding siblings ...) 2019-08-28 21:57 ` [PATCH v3 4/4] git-gui: allow undoing last revert Pratyush Yadav @ 2019-09-10 19:21 ` Pratyush Yadav 2019-09-10 20:26 ` Johannes Sixt 4 siblings, 1 reply; 49+ messages in thread From: Pratyush Yadav @ 2019-09-10 19:21 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Sixt, Bert Wesarg Johannes, Bert, All, If there are no further objections with the series, I will merge it in. On 29/08/19 03:27AM, Pratyush Yadav wrote: > Hi, > > This series adds the ability to revert selected lines and hunks in > git-gui. Partially based on the patch by Bert Wesarg [0]. > > The commits can be found in the topic branch 'py/revert-hunks-lines' > at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines > > Changes in v3: > - Drop the confirmation dialog on partial reverts. It is still there for > full file reverts (which was the original behaviour). > - Allow undoing the last revert. > - Update the context menu button layout. In v2, the layout was: > Stage Hunk > Revert Hunk > Stage Lines > Revert Lines > > Now it is: > Stage Hunk > Stage Lines > ----------- > Revert Hunk > Revert Lines > Undo Last Revert > - Return early when applying a patch fails. This is useful for this > series because in that case we don't save a faulty patch in > last_revert, causing the same error to pop up when reverting the patch > that failed to apply in the first place. > > Changes in v2: > - Add an option to disable the revert confirmation prompt as suggested > by Johannes Sixt. > - Base the patches on Pat's git-gui tree instead of git.git. > > [0] > https://public-inbox.org/git/a9ba4550a29d7f3c653561e7029f0920bf8eb008.1326116492.git.bert.wesarg@googlemail.com/ > > Pratyush Yadav (4): > git-gui: allow reverting selected lines > git-gui: allow reverting selected hunk > git-gui: return early when patch fails to apply > git-gui: allow undoing last revert > > git-gui.sh | 57 +++++++++++++++++++++++++++++-- > lib/diff.tcl | 96 ++++++++++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 135 insertions(+), 18 deletions(-) > > -- > 2.21.0 > -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines 2019-09-10 19:21 ` [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav @ 2019-09-10 20:26 ` Johannes Sixt 2019-09-12 18:59 ` Bert Wesarg 0 siblings, 1 reply; 49+ messages in thread From: Johannes Sixt @ 2019-09-10 20:26 UTC (permalink / raw) To: Pratyush Yadav; +Cc: git, Junio C Hamano, Bert Wesarg Am 10.09.19 um 21:21 schrieb Pratyush Yadav: > If there are no further objections with the series, I will merge it in. No objections. I use it in production. -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines 2019-09-10 20:26 ` Johannes Sixt @ 2019-09-12 18:59 ` Bert Wesarg 0 siblings, 0 replies; 49+ messages in thread From: Bert Wesarg @ 2019-09-12 18:59 UTC (permalink / raw) To: Johannes Sixt; +Cc: Pratyush Yadav, Git Mailing List, Junio C Hamano On Tue, Sep 10, 2019 at 10:26 PM Johannes Sixt <j6t@kdbg.org> wrote: > > Am 10.09.19 um 21:21 schrieb Pratyush Yadav: > > If there are no further objections with the series, I will merge it in. > > No objections. I use it in production. yep, Since 2012 ;-) > > -- Hannes ^ permalink raw reply [flat|nested] 49+ messages in thread
end of thread, other threads:[~2019-10-23 19:00 UTC | newest] Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav 2019-08-19 21:41 ` [PATCH 1/3] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav 2019-08-19 21:41 ` [PATCH 2/3] git-gui: Add the ability to revert selected lines Pratyush Yadav 2019-08-20 19:21 ` Johannes Sixt 2019-08-20 19:29 ` Pratyush Yadav 2019-08-20 21:19 ` Johannes Sixt 2019-08-21 21:48 ` Pratyush Yadav 2019-08-23 13:01 ` Johannes Schindelin 2019-08-23 16:28 ` Junio C Hamano 2019-08-23 17:03 ` Pratyush Yadav 2019-08-23 19:17 ` Johannes Sixt 2019-08-19 21:41 ` [PATCH 3/3] git-gui: Add the ability to revert selected hunk Pratyush Yadav 2019-08-20 18:47 ` [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Junio C Hamano 2019-08-20 19:49 ` Pratyush Yadav 2019-08-21 7:06 ` Bert Wesarg 2019-08-21 21:30 ` Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 1/4] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 2/4] git-gui: Add option to disable the revert confirmation prompt Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 3/4] git-gui: Add the ability to revert selected lines Pratyush Yadav 2019-08-23 6:29 ` Bert Wesarg 2019-08-23 16:51 ` Pratyush Yadav 2019-08-22 22:01 ` [PATCH v2 4/4] git-gui: Add the ability to revert selected hunk Pratyush Yadav 2019-08-22 22:34 ` [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines Junio C Hamano 2019-08-22 22:51 ` Pratyush Yadav 2019-08-23 6:04 ` Bert Wesarg 2019-08-23 16:04 ` Junio C Hamano 2019-08-23 16:44 ` Pratyush Yadav [not found] ` <CAKPyHN0QbCDzcG8=zPP4-WHKqMk3R0sJ0BjXDR=zak3OfEa2bg@mail.gmail.com> 2019-08-25 11:57 ` Pratyush Yadav 2019-08-23 23:43 ` David Aguilar 2019-08-24 6:54 ` Bert Wesarg 2019-08-24 6:57 ` Bert Wesarg 2019-08-24 7:26 ` David Aguilar 2019-08-24 8:09 ` Johannes Sixt 2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 1/4] git-gui: allow reverting selected lines Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 2/4] git-gui: allow reverting selected hunk Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 3/4] git-gui: return early when patch fails to apply Pratyush Yadav 2019-08-28 21:57 ` [PATCH v3 4/4] git-gui: allow undoing last revert Pratyush Yadav 2019-10-21 9:16 ` Bert Wesarg 2019-10-21 19:04 ` Pratyush Yadav 2019-10-22 8:17 ` Bert Wesarg 2019-10-23 18:57 ` Pratyush Yadav 2019-10-21 19:35 ` Johannes Sixt 2019-10-22 7:46 ` Bert Wesarg 2019-10-23 19:00 ` Pratyush Yadav 2019-09-10 19:21 ` [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav 2019-09-10 20:26 ` Johannes Sixt 2019-09-12 18:59 ` Bert Wesarg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.