From: Pratyush Yadav <me@yadavpratyush.com>
To: Bert Wesarg <bert.wesarg@googlemail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>, Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH v2 3/4] git-gui: Add the ability to revert selected lines
Date: Fri, 23 Aug 2019 22:21:21 +0530 [thread overview]
Message-ID: <20190823165121.2gklr4lz6mksdfc6@localhost.localdomain> (raw)
In-Reply-To: <CAKPyHN2GMV2n7rPO7NU7=wayNo3tYPNUhhwSuXK6EqAUbWEgng@mail.gmail.com>
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
next prev parent reply other threads:[~2019-08-23 16:51 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190823165121.2gklr4lz6mksdfc6@localhost.localdomain \
--to=me@yadavpratyush.com \
--cc=bert.wesarg@googlemail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).