* [PATCH] gitk: added external diff file rename detection
@ 2024-08-22 9:27 ToBoMi via GitGitGadget
2024-09-06 7:28 ` [PATCH v2] " ToBoMi via GitGitGadget
0 siblings, 1 reply; 13+ messages in thread
From: ToBoMi via GitGitGadget @ 2024-08-22 9:27 UTC (permalink / raw)
To: git; +Cc: ToBoMi, deboeto
From: deboeto <tobias.boesch@miele.com>
* If a file was renamed between commits and
an external diff is started through gitk
on the THE ORIGINAL FILE NAME (not the
renamed one), gitk was unable to open
the renamed file in the external diff
editor.
It failed to fetch the renamed file from
git, because it fetched it with the original
path in contrast to using the renamed path
* gitk now detects the rename and opens the
external diff with the original and the RENAMED
file instead of no file (it is able to fetch
the renamed file now from git with the renamed
path/filename)
* Since git doesn't destinguish between move or
rename this also works for moved files
* External diff detection and usage is optional
and has to be enabled in gitk settings
* External rename detection ist marked
EXPERIMENTAL in the settings and disabled
by default
* Showing the renamed file doesn't work when THE
RENAMED FILE is selected in gitk and an
external diff ist started on that file,
because the selected file is not renamed in
that commit. It already IS the renamed file.
Signed-off-by: deboeto <tobias.boesch@miele.com>
---
gitk: added external diff file rename detection
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1774/ToBoMi/detect_renamed_files_when_opening_diff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1774
gitk-git/gitk | 54 ++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 7 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 7a087f123d7..f7427f6d3f2 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3662,11 +3662,33 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc check_for_renames_in_diff {filepath} {
+ global ctext
+
+ set renamed_filenames [list {}]
+ set filename [file tail $filepath]
+ set rename_from_text_length 12
+ set rename_to_text_length 10
+ set reg_expr_rename_from {^rename from (.*$filename)}
+ set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
+ set reg_expr_rename_to {^rename to (.*)}
+ set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
+ if { ($rename_from_text_index != {})} {
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
+ if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_length chars" "$rename_from_text_index lineend"]
+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_length chars" "$rename_to_text_index lineend"]
+ }
+ }
+ return $renamed_filenames
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
global diffids
global extdifftool
+ global file_rename_detection
if {[llength $diffids] == 1} {
# no reference commit given
@@ -3692,8 +3714,21 @@ proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ if {$file_rename_detection} {
+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
@@ -11577,7 +11612,7 @@ proc create_prefs_page {w} {
proc prefspage_general {notebook} {
global NS maxwidth maxgraphpct showneartags showlocalchanges
global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
- global hideremotes want_ttk have_ttk maxrefs web_browser
+ global hideremotes want_ttk have_ttk maxrefs web_browser file_rename_detection
set page [create_prefs_page $notebook.general]
@@ -11639,12 +11674,16 @@ proc prefspage_general {notebook} {
grid $page.lgen - -sticky w -pady 10
${NS}::checkbutton $page.want_ttk -variable want_ttk \
-text [mc "Use themed widgets"]
+ ${NS}::checkbutton $page.file_rename_detection -variable file_rename_detection \
+ -text [mc "Use ext diff file rename detection"]
+ ${NS}::label $page.file_rename_detection_note -text [mc "(EXPERIMENTAL\nTries to find the file path of a\nrenamed file in external diff)"]
if {$have_ttk} {
${NS}::label $page.ttk_note -text [mc "(change requires restart)"]
} else {
${NS}::label $page.ttk_note -text [mc "(currently unavailable)"]
}
grid x $page.want_ttk $page.ttk_note -sticky w
+ grid x $page.file_rename_detection $page.file_rename_detection_note -sticky w
return $page
}
@@ -11725,7 +11764,7 @@ proc doprefs {} {
global oldprefs prefstop showneartags showlocalchanges
global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
- global hideremotes want_ttk have_ttk
+ global hideremotes want_ttk have_ttk file_rename_detection
set top .gitkprefs
set prefstop $top
@@ -11734,7 +11773,7 @@ proc doprefs {} {
return
}
foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+ limitdiffs tabstop perfile_attrs hideremotes want_ttk file_rename_detection} {
set oldprefs($v) [set $v]
}
ttk_toplevel $top
@@ -11860,7 +11899,7 @@ proc prefscan {} {
global oldprefs prefstop
foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+ limitdiffs tabstop perfile_attrs hideremotes want_ttk file_rename_detection} {
global $v
set $v $oldprefs($v)
}
@@ -12404,6 +12443,7 @@ set autoselect 1
set autosellen 40
set perfile_attrs 0
set want_ttk 1
+set file_rename_detection 0
if {[tk windowingsystem] eq "aqua"} {
set extdifftool "opendiff"
@@ -12498,7 +12538,7 @@ config_check_tmp_exists 50
set config_variables {
mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
cmitmode wrapcomment autoselect autosellen showneartags maxrefs visiblerefs
- hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk
+ hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk file_rename_detection
bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
base-commit: b9849e4f7631d80f146d159bf7b60263b3205632
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2] gitk: added external diff file rename detection
2024-08-22 9:27 [PATCH] gitk: added external diff file rename detection ToBoMi via GitGitGadget
@ 2024-09-06 7:28 ` ToBoMi via GitGitGadget
2024-10-02 9:47 ` AW: " tobias.boesch
2025-03-04 13:01 ` [PATCH v3] " ToBoMi via GitGitGadget
0 siblings, 2 replies; 13+ messages in thread
From: ToBoMi via GitGitGadget @ 2024-09-06 7:28 UTC (permalink / raw)
To: git; +Cc: ToBoMi, Tobias Boesch
From: Tobias Boesch <tobias.boesch@miele.com>
* If a file was renamed between commits and an external diff is started
through gitk on the THE ORIGINAL FILE NAME (not the renamed one),
gitk was unable to open the renamed file in the external diff editor.
It failed to fetch the renamed file from git, because it fetched it
with the original path in contrast to using the renamed path
* gitk now detects the rename and opens the external diff with the
original and the RENAMED file instead of no file (it is able to
fetch the renamed file now from git with the renamed path/filename)
* Since git doesn't destinguish between move or rename this also works
for moved files
* External diff detection and usage is optional and has to be enabled in
gitk settings
* External rename detection ist marked EXPERIMENTAL in the settings
and disabled by default
* Showing the renamed file doesn't work when THE RENAMED FILE is selected
in gitk and an external diff ist started on that file, because the
selected file is not renamed in that commit. It already IS the renamed
file.
Signed-off-by: Tobias Boeesch <tobias.boesch@miele.com>
---
gitk: added external diff file rename detection
Changes since v1:
* Commit message ident
* Commit message line length
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1774/ToBoMi/detect_renamed_files_when_opening_diff-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1774
Range-diff vs v1:
1: 4ff4aec82fe ! 1: 6209080cad6 gitk: added external diff file rename detection
@@
## Metadata ##
-Author: deboeto <tobias.boesch@miele.com>
+Author: Tobias Boesch <tobias.boesch@miele.com>
## Commit message ##
gitk: added external diff file rename detection
- * If a file was renamed between commits and
- an external diff is started through gitk
- on the THE ORIGINAL FILE NAME (not the
- renamed one), gitk was unable to open
- the renamed file in the external diff
- editor.
- It failed to fetch the renamed file from
- git, because it fetched it with the original
- path in contrast to using the renamed path
- * gitk now detects the rename and opens the
- external diff with the original and the RENAMED
- file instead of no file (it is able to fetch
- the renamed file now from git with the renamed
- path/filename)
- * Since git doesn't destinguish between move or
- rename this also works for moved files
- * External diff detection and usage is optional
- and has to be enabled in gitk settings
- * External rename detection ist marked
- EXPERIMENTAL in the settings and disabled
- by default
- * Showing the renamed file doesn't work when THE
- RENAMED FILE is selected in gitk and an
- external diff ist started on that file,
- because the selected file is not renamed in
- that commit. It already IS the renamed file.
+ * If a file was renamed between commits and an external diff is started
+ through gitk on the THE ORIGINAL FILE NAME (not the renamed one),
+ gitk was unable to open the renamed file in the external diff editor.
+ It failed to fetch the renamed file from git, because it fetched it
+ with the original path in contrast to using the renamed path
+ * gitk now detects the rename and opens the external diff with the
+ original and the RENAMED file instead of no file (it is able to
+ fetch the renamed file now from git with the renamed path/filename)
+ * Since git doesn't destinguish between move or rename this also works
+ for moved files
+ * External diff detection and usage is optional and has to be enabled in
+ gitk settings
+ * External rename detection ist marked EXPERIMENTAL in the settings
+ and disabled by default
+ * Showing the renamed file doesn't work when THE RENAMED FILE is selected
+ in gitk and an external diff ist started on that file, because the
+ selected file is not renamed in that commit. It already IS the renamed
+ file.
- Signed-off-by: deboeto <tobias.boesch@miele.com>
+ Signed-off-by: Tobias Boeesch <tobias.boesch@miele.com>
## gitk-git/gitk ##
@@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
gitk-git/gitk | 54 ++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 7 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 7a087f123d7..f7427f6d3f2 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3662,11 +3662,33 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc check_for_renames_in_diff {filepath} {
+ global ctext
+
+ set renamed_filenames [list {}]
+ set filename [file tail $filepath]
+ set rename_from_text_length 12
+ set rename_to_text_length 10
+ set reg_expr_rename_from {^rename from (.*$filename)}
+ set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
+ set reg_expr_rename_to {^rename to (.*)}
+ set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
+ if { ($rename_from_text_index != {})} {
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
+ if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_length chars" "$rename_from_text_index lineend"]
+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_length chars" "$rename_to_text_index lineend"]
+ }
+ }
+ return $renamed_filenames
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
global diffids
global extdifftool
+ global file_rename_detection
if {[llength $diffids] == 1} {
# no reference commit given
@@ -3692,8 +3714,21 @@ proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ if {$file_rename_detection} {
+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
@@ -11577,7 +11612,7 @@ proc create_prefs_page {w} {
proc prefspage_general {notebook} {
global NS maxwidth maxgraphpct showneartags showlocalchanges
global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
- global hideremotes want_ttk have_ttk maxrefs web_browser
+ global hideremotes want_ttk have_ttk maxrefs web_browser file_rename_detection
set page [create_prefs_page $notebook.general]
@@ -11639,12 +11674,16 @@ proc prefspage_general {notebook} {
grid $page.lgen - -sticky w -pady 10
${NS}::checkbutton $page.want_ttk -variable want_ttk \
-text [mc "Use themed widgets"]
+ ${NS}::checkbutton $page.file_rename_detection -variable file_rename_detection \
+ -text [mc "Use ext diff file rename detection"]
+ ${NS}::label $page.file_rename_detection_note -text [mc "(EXPERIMENTAL\nTries to find the file path of a\nrenamed file in external diff)"]
if {$have_ttk} {
${NS}::label $page.ttk_note -text [mc "(change requires restart)"]
} else {
${NS}::label $page.ttk_note -text [mc "(currently unavailable)"]
}
grid x $page.want_ttk $page.ttk_note -sticky w
+ grid x $page.file_rename_detection $page.file_rename_detection_note -sticky w
return $page
}
@@ -11725,7 +11764,7 @@ proc doprefs {} {
global oldprefs prefstop showneartags showlocalchanges
global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
- global hideremotes want_ttk have_ttk
+ global hideremotes want_ttk have_ttk file_rename_detection
set top .gitkprefs
set prefstop $top
@@ -11734,7 +11773,7 @@ proc doprefs {} {
return
}
foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+ limitdiffs tabstop perfile_attrs hideremotes want_ttk file_rename_detection} {
set oldprefs($v) [set $v]
}
ttk_toplevel $top
@@ -11860,7 +11899,7 @@ proc prefscan {} {
global oldprefs prefstop
foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+ limitdiffs tabstop perfile_attrs hideremotes want_ttk file_rename_detection} {
global $v
set $v $oldprefs($v)
}
@@ -12404,6 +12443,7 @@ set autoselect 1
set autosellen 40
set perfile_attrs 0
set want_ttk 1
+set file_rename_detection 0
if {[tk windowingsystem] eq "aqua"} {
set extdifftool "opendiff"
@@ -12498,7 +12538,7 @@ config_check_tmp_exists 50
set config_variables {
mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
cmitmode wrapcomment autoselect autosellen showneartags maxrefs visiblerefs
- hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk
+ hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk file_rename_detection
bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
base-commit: 2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread
* AW: [PATCH v2] gitk: added external diff file rename detection
2024-09-06 7:28 ` [PATCH v2] " ToBoMi via GitGitGadget
@ 2024-10-02 9:47 ` tobias.boesch
2025-03-04 13:01 ` [PATCH v3] " ToBoMi via GitGitGadget
1 sibling, 0 replies; 13+ messages in thread
From: tobias.boesch @ 2024-10-02 9:47 UTC (permalink / raw)
To: ToBoMi via GitGitGadget, git@vger.kernel.org
> -----Ursprüngliche Nachricht-----
> Von: ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Gesendet: Freitag, 6. September 2024 09:28
> An: git@vger.kernel.org
> Cc: Boesch, Tobias <tobias.boesch@miele.com>; Boesch, Tobias
> <tobias.boesch@miele.com>
> Betreff: [PATCH v2] gitk: added external diff file rename detection
>
> From: Tobias Boesch <tobias.boesch@miele.com>
>
> * If a file was renamed between commits and an external diff is started
> through gitk on the THE ORIGINAL FILE NAME (not the renamed one),
> gitk was unable to open the renamed file in the external diff editor.
> It failed to fetch the renamed file from git, because it fetched it
> with the original path in contrast to using the renamed path
> * gitk now detects the rename and opens the external diff with the
> original and the RENAMED file instead of no file (it is able to
> fetch the renamed file now from git with the renamed path/filename)
> * Since git doesn't destinguish between move or rename this also works
> for moved files
> * External diff detection and usage is optional and has to be enabled in
> gitk settings
> * External rename detection ist marked EXPERIMENTAL in the settings
> and disabled by default
It would be nice to have this without an additional setting.
Just enable the feature and remove the option.
> * Showing the renamed file doesn't work when THE RENAMED FILE is selected
> in gitk and an external diff ist started on that file, because the
> selected file is not renamed in that commit. It already IS the renamed
> file.
gitk already shows the exact same diff for both file entries, the file that is renamed and the renamed file.
This is why it would be okay to show the same external diff for both file entries.
It should be possible from the current code change to also gather the file names the other way round, that is to gather the file path of the file to be renamed from the renamed file path.
Since gitk doesn't change the diff depending on the file that is selected, the external diff should not switch sides depending on the file being selected.
>
> Signed-off-by: Tobias Boeesch <tobias.boesch@miele.com>
> ---
> gitk: added external diff file rename detection
>
> Changes since v1:
>
> * Commit message ident
> * Commit message line length
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-
> 1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-
> 1774/ToBoMi/detect_renamed_files_when_opening_diff-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1774
>
> Range-diff vs v1:
>
> 1: 4ff4aec82fe ! 1: 6209080cad6 gitk: added external diff file rename
> detection
> @@
> ## Metadata ##
> -Author: deboeto <tobias.boesch@miele.com>
> +Author: Tobias Boesch <tobias.boesch@miele.com>
>
> ## Commit message ##
> gitk: added external diff file rename detection
>
> - * If a file was renamed between commits and
> - an external diff is started through gitk
> - on the THE ORIGINAL FILE NAME (not the
> - renamed one), gitk was unable to open
> - the renamed file in the external diff
> - editor.
> - It failed to fetch the renamed file from
> - git, because it fetched it with the original
> - path in contrast to using the renamed path
> - * gitk now detects the rename and opens the
> - external diff with the original and the RENAMED
> - file instead of no file (it is able to fetch
> - the renamed file now from git with the renamed
> - path/filename)
> - * Since git doesn't destinguish between move or
> - rename this also works for moved files
> - * External diff detection and usage is optional
> - and has to be enabled in gitk settings
> - * External rename detection ist marked
> - EXPERIMENTAL in the settings and disabled
> - by default
> - * Showing the renamed file doesn't work when THE
> - RENAMED FILE is selected in gitk and an
> - external diff ist started on that file,
> - because the selected file is not renamed in
> - that commit. It already IS the renamed file.
> + * If a file was renamed between commits and an external diff is started
> + through gitk on the THE ORIGINAL FILE NAME (not the renamed one),
> + gitk was unable to open the renamed file in the external diff editor.
> + It failed to fetch the renamed file from git, because it fetched it
> + with the original path in contrast to using the renamed path
> + * gitk now detects the rename and opens the external diff with the
> + original and the RENAMED file instead of no file (it is able to
> + fetch the renamed file now from git with the renamed path/filename)
> + * Since git doesn't destinguish between move or rename this also works
> + for moved files
> + * External diff detection and usage is optional and has to be enabled in
> + gitk settings
> + * External rename detection ist marked EXPERIMENTAL in the settings
> + and disabled by default
> + * Showing the renamed file doesn't work when THE RENAMED FILE is
> selected
> + in gitk and an external diff ist started on that file, because the
> + selected file is not renamed in that commit. It already IS the renamed
> + file.
>
> - Signed-off-by: deboeto <tobias.boesch@miele.com>
> + Signed-off-by: Tobias Boeesch <tobias.boesch@miele.com>
>
> ## gitk-git/gitk ##
> @@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
>
>
> gitk-git/gitk | 54 ++++++++++++++++++++++++++++++++++++++++++++--
> -----
> 1 file changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk index 7a087f123d7..f7427f6d3f2
> 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -3662,11 +3662,33 @@ proc external_diff_get_one_file {diffid filename
> diffdir} {
> "revision $diffid"]
> }
>
> +proc check_for_renames_in_diff {filepath} {
> + global ctext
> +
> + set renamed_filenames [list {}]
> + set filename [file tail $filepath]
> + set rename_from_text_length 12
> + set rename_to_text_length 10
> + set reg_expr_rename_from {^rename from (.*$filename)}
> + set reg_expr_rename_from [subst -nobackslashes -nocommands
> $reg_expr_rename_from]
> + set reg_expr_rename_to {^rename to (.*)}
> + set rename_from_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_from 0.0]
> + if { ($rename_from_text_index != {})} {
> + set rename_to_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_to $rename_from_text_index]
> + if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) }
> {
> + lappend renamed_filenames [$ctext get "$rename_from_text_index +
> $rename_from_text_length chars" "$rename_from_text_index lineend"]
> + lappend renamed_filenames [$ctext get "$rename_to_text_index +
> $rename_to_text_length chars" "$rename_to_text_index lineend"]
> + }
> + }
> + return $renamed_filenames
> +}
> +
> proc external_diff {} {
> global nullid nullid2
> global flist_menu_file
> global diffids
> global extdifftool
> + global file_rename_detection
>
> if {[llength $diffids] == 1} {
> # no reference commit given
> @@ -3692,8 +3714,21 @@ proc external_diff {} {
> if {$diffdir eq {}} return
>
> # gather files to diff
> - set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> - set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> + if {$file_rename_detection} {
> + set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
> + set rename_from_filename [lindex $renamed_filenames 1]
> + set rename_to_filename [lindex $renamed_filenames 2]
> + if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
> + set difffromfile [external_diff_get_one_file $diffidfrom
> $rename_from_filename $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $rename_to_filename
> $diffdir]
> + } else {
> + set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $flist_menu_file
> $diffdir]
> + }
> + } else {
> + set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $flist_menu_file
> $diffdir]
> + }
>
> if {$difffromfile ne {} && $difftofile ne {}} {
> set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile] @@ -
> 11577,7 +11612,7 @@ proc create_prefs_page {w} { proc prefspage_general
> {notebook} {
> global NS maxwidth maxgraphpct showneartags showlocalchanges
> global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
> - global hideremotes want_ttk have_ttk maxrefs web_browser
> + global hideremotes want_ttk have_ttk maxrefs web_browser
> + file_rename_detection
>
> set page [create_prefs_page $notebook.general]
>
> @@ -11639,12 +11674,16 @@ proc prefspage_general {notebook} {
> grid $page.lgen - -sticky w -pady 10
> ${NS}::checkbutton $page.want_ttk -variable want_ttk \
> -text [mc "Use themed widgets"]
> + ${NS}::checkbutton $page.file_rename_detection -variable
> file_rename_detection \
> + -text [mc "Use ext diff file rename detection"]
> + ${NS}::label $page.file_rename_detection_note -text [mc
> + "(EXPERIMENTAL\nTries to find the file path of a\nrenamed file in
> + external diff)"]
> if {$have_ttk} {
> ${NS}::label $page.ttk_note -text [mc "(change requires restart)"]
> } else {
> ${NS}::label $page.ttk_note -text [mc "(currently unavailable)"]
> }
> grid x $page.want_ttk $page.ttk_note -sticky w
> + grid x $page.file_rename_detection $page.file_rename_detection_note
> + -sticky w
> return $page
> }
>
> @@ -11725,7 +11764,7 @@ proc doprefs {} {
> global oldprefs prefstop showneartags showlocalchanges
> global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
> global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
> - global hideremotes want_ttk have_ttk
> + global hideremotes want_ttk have_ttk file_rename_detection
>
> set top .gitkprefs
> set prefstop $top
> @@ -11734,7 +11773,7 @@ proc doprefs {} {
> return
> }
> foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
> - limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
> + limitdiffs tabstop perfile_attrs hideremotes
> + want_ttk file_rename_detection} {
> set oldprefs($v) [set $v]
> }
> ttk_toplevel $top
> @@ -11860,7 +11899,7 @@ proc prefscan {} {
> global oldprefs prefstop
>
> foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
> - limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
> + limitdiffs tabstop perfile_attrs hideremotes
> + want_ttk file_rename_detection} {
> global $v
> set $v $oldprefs($v)
> }
> @@ -12404,6 +12443,7 @@ set autoselect 1 set autosellen 40 set
> perfile_attrs 0 set want_ttk 1
> +set file_rename_detection 0
>
> if {[tk windowingsystem] eq "aqua"} {
> set extdifftool "opendiff"
> @@ -12498,7 +12538,7 @@ config_check_tmp_exists 50 set
> config_variables {
> mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
> cmitmode wrapcomment autoselect autosellen showneartags maxrefs
> visiblerefs
> - hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk
> + hideremotes showlocalchanges datetimeformat limitdiffs uicolor
> + want_ttk file_rename_detection
> bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
> markbgcolor diffcontext selectbgcolor foundbgcolor
> currentsearchhitbgcolor
> extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
>
> base-commit: 2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c
> --
> gitgitgadget
-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] gitk: added external diff file rename detection
2024-09-06 7:28 ` [PATCH v2] " ToBoMi via GitGitGadget
2024-10-02 9:47 ` AW: " tobias.boesch
@ 2025-03-04 13:01 ` ToBoMi via GitGitGadget
2025-03-16 16:21 ` Johannes Sixt
2025-04-28 8:47 ` [PATCH v4] gitk: add " ToBoMi via GitGitGadget
1 sibling, 2 replies; 13+ messages in thread
From: ToBoMi via GitGitGadget @ 2025-03-04 13:01 UTC (permalink / raw)
To: git; +Cc: ToBoMi, Tobias Boesch
From: Tobias Boesch <tobias.boesch@miele.com>
* If a file was renamed between commits and an external diff is started
through gitk on the original or the renamed file name,
gitk was unable to open the renamed file in the external diff editor.
It failed to fetch the renamed file from git, because it fetched it
using its original path in contrast to using the renamed path of the
file.
* With this change gitk detects the rename and opens the external diff
with the original and the renamed file instead of no file (it is able
to fetch the renamed file path and name now from git).
* Since git doesn't destinguish between move or rename this also works
for moved files.
* Showing the external diff with the original and the renamed file
works when either of the files is selected in gitk.
Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
---
gitk: added external diff file rename detection
Changes since v1:
* Commit message ident
* Commit message line length
Changes since v2:
* Removed option for rename detection (Adding GUI options seems to be
not desired - which is understandable)
* Rebased on current master of git-for-windows
* Renamed variables for a better understanding
* Made rename detection also work when the renamed file is selected in
gitk
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1774/ToBoMi/detect_renamed_files_when_opening_diff-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1774
Range-diff vs v2:
1: 6209080cad6 ! 1: 1a64e989713 gitk: added external diff file rename detection
@@ Commit message
gitk: added external diff file rename detection
* If a file was renamed between commits and an external diff is started
- through gitk on the THE ORIGINAL FILE NAME (not the renamed one),
+ through gitk on the original or the renamed file name,
gitk was unable to open the renamed file in the external diff editor.
It failed to fetch the renamed file from git, because it fetched it
- with the original path in contrast to using the renamed path
- * gitk now detects the rename and opens the external diff with the
- original and the RENAMED file instead of no file (it is able to
- fetch the renamed file now from git with the renamed path/filename)
- * Since git doesn't destinguish between move or rename this also works
- for moved files
- * External diff detection and usage is optional and has to be enabled in
- gitk settings
- * External rename detection ist marked EXPERIMENTAL in the settings
- and disabled by default
- * Showing the renamed file doesn't work when THE RENAMED FILE is selected
- in gitk and an external diff ist started on that file, because the
- selected file is not renamed in that commit. It already IS the renamed
+ using its original path in contrast to using the renamed path of the
file.
+ * With this change gitk detects the rename and opens the external diff
+ with the original and the renamed file instead of no file (it is able
+ to fetch the renamed file path and name now from git).
+ * Since git doesn't destinguish between move or rename this also works
+ for moved files.
+ * Showing the external diff with the original and the renamed file
+ works when either of the files is selected in gitk.
- Signed-off-by: Tobias Boeesch <tobias.boesch@miele.com>
+ Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
## gitk-git/gitk ##
@@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
@@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
+
+ set renamed_filenames [list {}]
+ set filename [file tail $filepath]
-+ set rename_from_text_length 12
-+ set rename_to_text_length 10
++ set rename_from_text_identifier_length 12
++ set rename_to_text_identifier_length 10
+ set reg_expr_rename_from {^rename from (.*$filename)}
+ set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
-+ set reg_expr_rename_to {^rename to (.*)}
+ set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
+ if { ($rename_from_text_index != {})} {
++ set reg_expr_rename_to {^rename to (.*)}
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
+ if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
-+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_length chars" "$rename_from_text_index lineend"]
-+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_length chars" "$rename_to_text_index lineend"]
++ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
++ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
++ }
++ return $renamed_filenames
++ }
++ set reg_expr_rename_to {^rename to (.*$filename)}
++ set reg_expr_rename_to [subst -nobackslashes -nocommands $reg_expr_rename_to]
++ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to 0.0]
++ if { ($rename_to_text_index != {})} {
++ set reg_expr_rename_from {^rename from (.*)}
++ set rename_from_text_index [$ctext search -backwards -elide -regexp -- $reg_expr_rename_from $rename_to_text_index]
++ if { ($rename_to_text_index != {}) && ($rename_from_text_index != {}) } {
++ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
++ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
+ }
++ return $renamed_filenames
+ }
-+ return $renamed_filenames
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
- global diffids
- global extdifftool
-+ global file_rename_detection
-
- if {[llength $diffids] == 1} {
- # no reference commit given
@@ gitk-git/gitk: proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
-+ if {$file_rename_detection} {
-+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
-+ set rename_from_filename [lindex $renamed_filenames 1]
-+ set rename_to_filename [lindex $renamed_filenames 2]
-+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
-+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
-+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
-+ } else {
-+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
-+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
-+ }
++ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
++ set rename_from_filename [lindex $renamed_filenames 1]
++ set rename_to_filename [lindex $renamed_filenames 2]
++ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
++ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
++ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
@@ gitk-git/gitk: proc external_diff {} {
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
-@@ gitk-git/gitk: proc create_prefs_page {w} {
- proc prefspage_general {notebook} {
- global NS maxwidth maxgraphpct showneartags showlocalchanges
- global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
-- global hideremotes want_ttk have_ttk maxrefs web_browser
-+ global hideremotes want_ttk have_ttk maxrefs web_browser file_rename_detection
-
- set page [create_prefs_page $notebook.general]
-
-@@ gitk-git/gitk: proc prefspage_general {notebook} {
- grid $page.lgen - -sticky w -pady 10
- ${NS}::checkbutton $page.want_ttk -variable want_ttk \
- -text [mc "Use themed widgets"]
-+ ${NS}::checkbutton $page.file_rename_detection -variable file_rename_detection \
-+ -text [mc "Use ext diff file rename detection"]
-+ ${NS}::label $page.file_rename_detection_note -text [mc "(EXPERIMENTAL\nTries to find the file path of a\nrenamed file in external diff)"]
- if {$have_ttk} {
- ${NS}::label $page.ttk_note -text [mc "(change requires restart)"]
- } else {
- ${NS}::label $page.ttk_note -text [mc "(currently unavailable)"]
- }
- grid x $page.want_ttk $page.ttk_note -sticky w
-+ grid x $page.file_rename_detection $page.file_rename_detection_note -sticky w
- return $page
- }
-
-@@ gitk-git/gitk: proc doprefs {} {
- global oldprefs prefstop showneartags showlocalchanges
- global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
- global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
-- global hideremotes want_ttk have_ttk
-+ global hideremotes want_ttk have_ttk file_rename_detection
-
- set top .gitkprefs
- set prefstop $top
-@@ gitk-git/gitk: proc doprefs {} {
- return
- }
- foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
-- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
-+ limitdiffs tabstop perfile_attrs hideremotes want_ttk file_rename_detection} {
- set oldprefs($v) [set $v]
- }
- ttk_toplevel $top
-@@ gitk-git/gitk: proc prefscan {} {
- global oldprefs prefstop
-
- foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
-- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
-+ limitdiffs tabstop perfile_attrs hideremotes want_ttk file_rename_detection} {
- global $v
- set $v $oldprefs($v)
- }
-@@ gitk-git/gitk: set autoselect 1
- set autosellen 40
- set perfile_attrs 0
- set want_ttk 1
-+set file_rename_detection 0
-
- if {[tk windowingsystem] eq "aqua"} {
- set extdifftool "opendiff"
-@@ gitk-git/gitk: config_check_tmp_exists 50
- set config_variables {
- mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
- cmitmode wrapcomment autoselect autosellen showneartags maxrefs visiblerefs
-- hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk
-+ hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk file_rename_detection
- bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
- markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
- extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
gitk-git/gitk | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index bc9efa18566..ddbe60398f2 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3806,6 +3806,39 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc check_for_renames_in_diff {filepath} {
+ global ctext
+
+ set renamed_filenames [list {}]
+ set filename [file tail $filepath]
+ set rename_from_text_identifier_length 12
+ set rename_to_text_identifier_length 10
+ set reg_expr_rename_from {^rename from (.*$filename)}
+ set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
+ set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
+ if { ($rename_from_text_index != {})} {
+ set reg_expr_rename_to {^rename to (.*)}
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
+ if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
+ }
+ return $renamed_filenames
+ }
+ set reg_expr_rename_to {^rename to (.*$filename)}
+ set reg_expr_rename_to [subst -nobackslashes -nocommands $reg_expr_rename_to]
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to 0.0]
+ if { ($rename_to_text_index != {})} {
+ set reg_expr_rename_from {^rename from (.*)}
+ set rename_from_text_index [$ctext search -backwards -elide -regexp -- $reg_expr_rename_from $rename_to_text_index]
+ if { ($rename_to_text_index != {}) && ($rename_from_text_index != {}) } {
+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
+ }
+ return $renamed_filenames
+ }
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
@@ -3836,8 +3869,16 @@ proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
base-commit: db91954e18654eeebc54c900f44c704002e1866d
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] gitk: added external diff file rename detection
2025-03-04 13:01 ` [PATCH v3] " ToBoMi via GitGitGadget
@ 2025-03-16 16:21 ` Johannes Sixt
2025-04-28 8:52 ` AW: " tobias.boesch
2025-04-28 8:47 ` [PATCH v4] gitk: add " ToBoMi via GitGitGadget
1 sibling, 1 reply; 13+ messages in thread
From: Johannes Sixt @ 2025-03-16 16:21 UTC (permalink / raw)
To: ToBoMi; +Cc: git, ToBoMi via GitGitGadget
Am 04.03.25 um 14:01 schrieb ToBoMi via GitGitGadget:
> From: Tobias Boesch <tobias.boesch@miele.com>
>
> * If a file was renamed between commits and an external diff is started
> through gitk on the original or the renamed file name,
> gitk was unable to open the renamed file in the external diff editor.
> It failed to fetch the renamed file from git, because it fetched it
> using its original path in contrast to using the renamed path of the
> file.
> * With this change gitk detects the rename and opens the external diff
> with the original and the renamed file instead of no file (it is able
> to fetch the renamed file path and name now from git).
> * Since git doesn't destinguish between move or rename this also works
> for moved files.
> * Showing the external diff with the original and the renamed file
> works when either of the files is selected in gitk.
>
> Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
> ---
I've picked up this patch, but haven't found time to look at it in
detail. It will take some time. Please bear with me.
The commit message would need to be rewritten to match our usual style:
- We do not use bullet points for normal text paragraphs.
- We describe the status quo in present tense,
- and then the changes in imperative mood. (Like, "code, become so!")
- The subject is not in past tense, but usually also imperative.
-- Hannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* AW: [PATCH v3] gitk: added external diff file rename detection
2025-03-16 16:21 ` Johannes Sixt
@ 2025-04-28 8:52 ` tobias.boesch
0 siblings, 0 replies; 13+ messages in thread
From: tobias.boesch @ 2025-04-28 8:52 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: ToBoMi via GitGitGadget
Thanks for the update.
Take the time you need.
Changes to the message are following.
> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Sonntag, 16. März 2025 17:22
> An: Boesch, Tobias <tobias.boesch@miele.com>
> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Betreff: Re: [PATCH v3] gitk: added external diff file rename detection
>
> Am 04.03.25 um 14:01 schrieb ToBoMi via GitGitGadget:
> > From: Tobias Boesch <tobias.boesch@miele.com>
> >
> > * If a file was renamed between commits and an external diff is started
> > through gitk on the original or the renamed file name,
> > gitk was unable to open the renamed file in the external diff editor.
> > It failed to fetch the renamed file from git, because it fetched it
> > using its original path in contrast to using the renamed path of the
> > file.
> > * With this change gitk detects the rename and opens the external diff
> > with the original and the renamed file instead of no file (it is able
> > to fetch the renamed file path and name now from git).
> > * Since git doesn't destinguish between move or rename this also works
> > for moved files.
> > * Showing the external diff with the original and the renamed file
> > works when either of the files is selected in gitk.
> >
> > Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
> > ---
>
> I've picked up this patch, but haven't found time to look at it in detail. It will
> take some time. Please bear with me.
>
> The commit message would need to be rewritten to match our usual style:
>
> - We do not use bullet points for normal text paragraphs.
> - We describe the status quo in present tense,
> - and then the changes in imperative mood. (Like, "code, become so!")
> - The subject is not in past tense, but usually also imperative.
>
> -- Hannes
-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4] gitk: add external diff file rename detection
2025-03-04 13:01 ` [PATCH v3] " ToBoMi via GitGitGadget
2025-03-16 16:21 ` Johannes Sixt
@ 2025-04-28 8:47 ` ToBoMi via GitGitGadget
2025-05-06 19:39 ` Johannes Sixt
2025-06-10 8:29 ` [PATCH v5] " ToBoMi via GitGitGadget
1 sibling, 2 replies; 13+ messages in thread
From: ToBoMi via GitGitGadget @ 2025-04-28 8:47 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, ToBoMi, Tobias Boesch
From: Tobias Boesch <tobias.boesch@miele.com>
If a file is renamed between commits and an external diff is started
through gitk on the original or the renamed file name,
gitk is unable to open the renamed file in the external diff editor.
It fails to fetch the renamed file from git, because it fetches it
using its original path in contrast to using the renamed path of the
file.
Detect the rename and open the external diff with the original and
the renamed file instead of no file (fetch the renamed file path and
name from git) no matter if the original or the renamed file is
selected in gitk.
Since moved or renamed file are handled the same way do this also
for moved files.
Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
---
gitk: add external diff file rename detection
Changes since v1:
* Commit message ident
* Commit message line length
Changes since v2:
* Removed option for rename detection (Adding GUI options seems to be
not desired - which is understandable)
* Rebased on current master of git-for-windows
* Renamed variables for a better understanding
* Made rename detection also work when the renamed file is selected in
gitk
Changes since v3:
* Changed message to use present tense, removed bullet points and
described changes in imperative mood
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1774/ToBoMi/detect_renamed_files_when_opening_diff-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1774
Range-diff vs v3:
1: 1a64e989713 ! 1: 948b94bef5c gitk: added external diff file rename detection
@@ Metadata
Author: Tobias Boesch <tobias.boesch@miele.com>
## Commit message ##
- gitk: added external diff file rename detection
+ gitk: add external diff file rename detection
- * If a file was renamed between commits and an external diff is started
- through gitk on the original or the renamed file name,
- gitk was unable to open the renamed file in the external diff editor.
- It failed to fetch the renamed file from git, because it fetched it
- using its original path in contrast to using the renamed path of the
- file.
- * With this change gitk detects the rename and opens the external diff
- with the original and the renamed file instead of no file (it is able
- to fetch the renamed file path and name now from git).
- * Since git doesn't destinguish between move or rename this also works
- for moved files.
- * Showing the external diff with the original and the renamed file
- works when either of the files is selected in gitk.
+ If a file is renamed between commits and an external diff is started
+ through gitk on the original or the renamed file name,
+ gitk is unable to open the renamed file in the external diff editor.
+ It fails to fetch the renamed file from git, because it fetches it
+ using its original path in contrast to using the renamed path of the
+ file.
+ Detect the rename and open the external diff with the original and
+ the renamed file instead of no file (fetch the renamed file path and
+ name from git) no matter if the original or the renamed file is
+ selected in gitk.
+ Since moved or renamed file are handled the same way do this also
+ for moved files.
Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
gitk-git/gitk | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index bc9efa18566..ddbe60398f2 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3806,6 +3806,39 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc check_for_renames_in_diff {filepath} {
+ global ctext
+
+ set renamed_filenames [list {}]
+ set filename [file tail $filepath]
+ set rename_from_text_identifier_length 12
+ set rename_to_text_identifier_length 10
+ set reg_expr_rename_from {^rename from (.*$filename)}
+ set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
+ set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
+ if { ($rename_from_text_index != {})} {
+ set reg_expr_rename_to {^rename to (.*)}
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
+ if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
+ }
+ return $renamed_filenames
+ }
+ set reg_expr_rename_to {^rename to (.*$filename)}
+ set reg_expr_rename_to [subst -nobackslashes -nocommands $reg_expr_rename_to]
+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to 0.0]
+ if { ($rename_to_text_index != {})} {
+ set reg_expr_rename_from {^rename from (.*)}
+ set rename_from_text_index [$ctext search -backwards -elide -regexp -- $reg_expr_rename_from $rename_to_text_index]
+ if { ($rename_to_text_index != {}) && ($rename_from_text_index != {}) } {
+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
+ }
+ return $renamed_filenames
+ }
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
@@ -3836,8 +3869,16 @@ proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4] gitk: add external diff file rename detection
2025-04-28 8:47 ` [PATCH v4] gitk: add " ToBoMi via GitGitGadget
@ 2025-05-06 19:39 ` Johannes Sixt
2025-06-10 8:28 ` AW: " tobias.boesch
2025-06-10 8:29 ` [PATCH v5] " ToBoMi via GitGitGadget
1 sibling, 1 reply; 13+ messages in thread
From: Johannes Sixt @ 2025-05-06 19:39 UTC (permalink / raw)
To: ToBoMi via GitGitGadget, git; +Cc: ToBoMi
Am 28.04.25 um 10:47 schrieb ToBoMi via GitGitGadget:
> From: Tobias Boesch <tobias.boesch@miele.com>
>
> If a file is renamed between commits and an external diff is started
> through gitk on the original or the renamed file name,
> gitk is unable to open the renamed file in the external diff editor.
> It fails to fetch the renamed file from git, because it fetches it
> using its original path in contrast to using the renamed path of the
> file.
> Detect the rename and open the external diff with the original and
> the renamed file instead of no file (fetch the renamed file path and
> name from git) no matter if the original or the renamed file is
> selected in gitk.
> Since moved or renamed file are handled the same way do this also
> for moved files.
>
> Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
Thank you. Sorry for taking so long to respond.
In general, I like the goal of this patch.
I am not familar, yet, how renamed files are represented in Gitk.
I wonder whether it is necessary to parse diff text to find renamed file
names. When you click on a renamed file in the file list, the diff panel
jumps to the corresponding text for both the original file name and the
renamed file name. Is the information about those two names not already
available?
Would it make sense to support also copied files?
> gitk-git/gitk | 45 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index bc9efa18566..ddbe60398f2 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -3806,6 +3806,39 @@ proc external_diff_get_one_file {diffid filename diffdir} {
> "revision $diffid"]
> }
>
> +proc check_for_renames_in_diff {filepath} {
> + global ctext
> +
> + set renamed_filenames [list {}]
> + set filename [file tail $filepath]
> + set rename_from_text_identifier_length 12
> + set rename_to_text_identifier_length 10
> + set reg_expr_rename_from {^rename from (.*$filename)}
$filename can certainly have characters that are special for a regular
expression, such as the fullstop, right? They need to be escaped or this
will find the wrong file if one at all.
If this search wants to find one side of the rename, why does it ignore
the directories?
> + set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
> + set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
> + if { ($rename_from_text_index != {})} {
Here and elsewhere in this patch we have a string comparison that uses
'!='. It should use 'ne'.
Please avoid the extra set of parentheses, even around && (below). Also,
in this code base, we do not have spaces around the condition inside {}.
> + set reg_expr_rename_to {^rename to (.*)}
> + set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
> + if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
> + lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
> + lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
> + }
> + return $renamed_filenames
> + }
> + set reg_expr_rename_to {^rename to (.*$filename)}
> + set reg_expr_rename_to [subst -nobackslashes -nocommands $reg_expr_rename_to]
> + set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to 0.0]
> + if { ($rename_to_text_index != {})} {
> + set reg_expr_rename_from {^rename from (.*)}
> + set rename_from_text_index [$ctext search -backwards -elide -regexp -- $reg_expr_rename_from $rename_to_text_index]
> + if { ($rename_to_text_index != {}) && ($rename_from_text_index != {}) } {
> + lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
> + lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
> + }
> + return $renamed_filenames
> + }
> +}
> +
Can we please have shorter variable names? They are all local variables.
I have to spend so mucht time to find the end of the variable names
before I can understand what the lines do...
> proc external_diff {} {
> global nullid nullid2
> global flist_menu_file
> @@ -3836,8 +3869,16 @@ proc external_diff {} {
> if {$diffdir eq {}} return
>
> # gather files to diff
> - set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
> - set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> + set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
> + set rename_from_filename [lindex $renamed_filenames 1]
> + set rename_to_filename [lindex $renamed_filenames 2]
> + if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
> + set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
> + } else {
> + set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> + }
>
> if {$difffromfile ne {} && $difftofile ne {}} {
> set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
>
> base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff
-- Hannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* AW: [PATCH v4] gitk: add external diff file rename detection
2025-05-06 19:39 ` Johannes Sixt
@ 2025-06-10 8:28 ` tobias.boesch
0 siblings, 0 replies; 13+ messages in thread
From: tobias.boesch @ 2025-06-10 8:28 UTC (permalink / raw)
To: Johannes Sixt, ToBoMi via GitGitGadget, git@vger.kernel.org
> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Dienstag, 6. Mai 2025 21:39
> An: ToBoMi via GitGitGadget <gitgitgadget@gmail.com>; git@vger.kernel.org
> Cc: Boesch, Tobias <tobias.boesch@miele.com>
> Betreff: Re: [PATCH v4] gitk: add external diff file rename detection
>
> Am 28.04.25 um 10:47 schrieb ToBoMi via GitGitGadget:
> > From: Tobias Boesch <tobias.boesch@miele.com>
> >
> > If a file is renamed between commits and an external diff is started
> > through gitk on the original or the renamed file name, gitk is unable
> > to open the renamed file in the external diff editor.
> > It fails to fetch the renamed file from git, because it fetches it
> > using its original path in contrast to using the renamed path of the
> > file.
> > Detect the rename and open the external diff with the original and the
> > renamed file instead of no file (fetch the renamed file path and name
> > from git) no matter if the original or the renamed file is selected in
> > gitk.
> > Since moved or renamed file are handled the same way do this also for
> > moved files.
> >
> > Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
>
> Thank you. Sorry for taking so long to respond.
>
> In general, I like the goal of this patch.
>
> I am not familar, yet, how renamed files are represented in Gitk.
>
> I wonder whether it is necessary to parse diff text to find renamed file names.
> When you click on a renamed file in the file list, the diff panel jumps to the
> corresponding text for both the original file name and the renamed file name.
> Is the information about those two names not already available?
I believe this could go wrong, because I think one can scroll after selecting
the file and then right click on it or even another file to execute the external diff.
Then the filenames are no longer at the top top of the text field.
I got around this by finding a git command that delivers the full paths of the original
and the renamed file. Since git commands are executed in the codebase already I
hope this is okay and maybe this is even more efficient than parsing the ctext.
>
> Would it make sense to support also copied files?
I don't know how why this is a benefit and I believe that git currently does not handle
copied files the way it does with renamed or moved files. That's why I would like to skip this.
>
> > gitk-git/gitk | 45 +++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/gitk-git/gitk b/gitk-git/gitk index
> > bc9efa18566..ddbe60398f2 100755
> > --- a/gitk-git/gitk
> > +++ b/gitk-git/gitk
> > @@ -3806,6 +3806,39 @@ proc external_diff_get_one_file {diffid filename
> diffdir} {
> > "revision $diffid"]
> > }
> >
> > +proc check_for_renames_in_diff {filepath} {
> > + global ctext
> > +
> > + set renamed_filenames [list {}]
> > + set filename [file tail $filepath]
> > + set rename_from_text_identifier_length 12
> > + set rename_to_text_identifier_length 10
> > + set reg_expr_rename_from {^rename from (.*$filename)}
>
> $filename can certainly have characters that are special for a regular
> expression, such as the fullstop, right? They need to be escaped or this will
> find the wrong file if one at all.
>
> If this search wants to find one side of the rename, why does it ignore the
> directories?
True. Now that a git command is used that emits the full path of the files
This regex is no longer necessary and is removed.
>
> > + set reg_expr_rename_from [subst -nobackslashes -nocommands
> $reg_expr_rename_from]
> > + set rename_from_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_from 0.0]
> > + if { ($rename_from_text_index != {})} {
>
> Here and elsewhere in this patch we have a string comparison that uses '!='. It
> should use 'ne'.
done
>
> Please avoid the extra set of parentheses, even around && (below). Also, in
> this code base, we do not have spaces around the condition inside {}.
done
>
> > + set reg_expr_rename_to {^rename to (.*)}
> > + set rename_to_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_to $rename_from_text_index]
> > + if { ($rename_from_text_index != {}) && ($rename_to_text_index != {})
> } {
> > + lappend renamed_filenames [$ctext get "$rename_from_text_index
> + $rename_from_text_identifier_length chars" "$rename_from_text_index
> lineend"]
> > + lappend renamed_filenames [$ctext get "$rename_to_text_index +
> $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
> > + }
> > + return $renamed_filenames
> > + }
> > + set reg_expr_rename_to {^rename to (.*$filename)}
> > + set reg_expr_rename_to [subst -nobackslashes -nocommands
> $reg_expr_rename_to]
> > + set rename_to_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_to 0.0]
> > + if { ($rename_to_text_index != {})} {
> > + set reg_expr_rename_from {^rename from (.*)}
> > + set rename_from_text_index [$ctext search -backwards -elide -regexp -
> - $reg_expr_rename_from $rename_to_text_index]
> > + if { ($rename_to_text_index != {}) && ($rename_from_text_index != {})
> } {
> > + lappend renamed_filenames [$ctext get "$rename_from_text_index
> + $rename_from_text_identifier_length chars" "$rename_from_text_index
> lineend"]
> > + lappend renamed_filenames [$ctext get "$rename_to_text_index +
> $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
> > + }
> > + return $renamed_filenames
> > + }
> > +}
> > +
>
> Can we please have shorter variable names? They are all local variables.
> I have to spend so mucht time to find the end of the variable names before I
> can understand what the lines do...
done
>
> > proc external_diff {} {
> > global nullid nullid2
> > global flist_menu_file
> > @@ -3836,8 +3869,16 @@ proc external_diff {} {
> > if {$diffdir eq {}} return
> >
> > # gather files to diff
> > - set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> > - set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> > + set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
> > + set rename_from_filename [lindex $renamed_filenames 1]
> > + set rename_to_filename [lindex $renamed_filenames 2]
> > + if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
> > + set difffromfile [external_diff_get_one_file $diffidfrom
> $rename_from_filename $diffdir]
> > + set difftofile [external_diff_get_one_file $diffidto $rename_to_filename
> $diffdir]
> > + } else {
> > + set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> > + set difftofile [external_diff_get_one_file $diffidto $flist_menu_file
> $diffdir]
> > + }
> >
> > if {$difffromfile ne {} && $difftofile ne {}} {
> > set cmd [list [shellsplit $extdifftool] $difffromfile
> > $difftofile]
> >
> > base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff
>
> -- Hannes
-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5] gitk: add external diff file rename detection
2025-04-28 8:47 ` [PATCH v4] gitk: add " ToBoMi via GitGitGadget
2025-05-06 19:39 ` Johannes Sixt
@ 2025-06-10 8:29 ` ToBoMi via GitGitGadget
2025-06-13 8:18 ` AW: " tobias.boesch
2025-06-24 9:05 ` [PATCH v6] " ToBoMi via GitGitGadget
1 sibling, 2 replies; 13+ messages in thread
From: ToBoMi via GitGitGadget @ 2025-06-10 8:29 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, tobias.boesch, ToBoMi, Tobias Boesch
From: Tobias Boesch <tobias.boesch@miele.com>
If a file is renamed between commits and an external diff is started
through gitk on the original or the renamed file name,
gitk is unable to open the renamed file in the external diff editor.
It fails to fetch the renamed file from git, because it fetches it
using its original path in contrast to using the renamed path of the
file.
Detect the rename and open the external diff with the original and
the renamed file instead of no file (fetch the renamed file path and
name from git) no matter if the original or the renamed file is
selected in gitk.
Since moved or renamed file are handled the same way do this also
for moved files.
Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
---
gitk: add external diff file rename detection
Changes since v1:
* Commit message ident
* Commit message line length
Changes since v2:
* Removed option for rename detection (Adding GUI options seems to be
not desired - which is understandable)
* Rebased on current master of git-for-windows
* Renamed variables for a better understanding
* Made rename detection also work when the renamed file is selected in
gitk
Changes since v3:
* Changed message to use present tense, removed bullet points and
described changes in imperative mood
Changes sine v4:
* Use a git command to gather the changed file paths rather than
parsing the text from the diff window panel for efficiency and to
avoid regex containing the filename as a variable.
* Change != to ne in string comparison
* removed extra set of parentheses around &&
* shorter variable names
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1774/ToBoMi/detect_renamed_files_when_opening_diff-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1774
Range-diff vs v4:
1: 948b94bef5c ! 1: 0d28f189dc3 gitk: add external diff file rename detection
@@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
-+proc check_for_renames_in_diff {filepath} {
-+ global ctext
++proc check_for_renames_in_diff {diffidfrom diffidto filepath} {
++ global nullid nullid2
+
-+ set renamed_filenames [list {}]
-+ set filename [file tail $filepath]
-+ set rename_from_text_identifier_length 12
-+ set rename_to_text_identifier_length 10
-+ set reg_expr_rename_from {^rename from (.*$filename)}
-+ set reg_expr_rename_from [subst -nobackslashes -nocommands $reg_expr_rename_from]
-+ set rename_from_text_index [$ctext search -elide -regexp -- $reg_expr_rename_from 0.0]
-+ if { ($rename_from_text_index != {})} {
-+ set reg_expr_rename_to {^rename to (.*)}
-+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to $rename_from_text_index]
-+ if { ($rename_from_text_index != {}) && ($rename_to_text_index != {}) } {
-+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
-+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
-+ }
-+ return $renamed_filenames
++ if {$diffidfrom eq $nullid} {
++ set rev [list $diffidto -R]
++ } elseif {$diffidfrom eq $nullid2} {
++ set rev [list $diffidto --cached -R]
++ } elseif {$diffidto eq $nullid} {
++ set rev [list $diffidfrom]
++ } elseif {$diffidto eq $nullid2} {
++ set rev [list $diffidfrom --cached]
++ } else {
++ set rev [list $diffidfrom..$diffidto]
+ }
-+ set reg_expr_rename_to {^rename to (.*$filename)}
-+ set reg_expr_rename_to [subst -nobackslashes -nocommands $reg_expr_rename_to]
-+ set rename_to_text_index [$ctext search -elide -regexp -- $reg_expr_rename_to 0.0]
-+ if { ($rename_to_text_index != {})} {
-+ set reg_expr_rename_from {^rename from (.*)}
-+ set rename_from_text_index [$ctext search -backwards -elide -regexp -- $reg_expr_rename_from $rename_to_text_index]
-+ if { ($rename_to_text_index != {}) && ($rename_from_text_index != {}) } {
-+ lappend renamed_filenames [$ctext get "$rename_from_text_index + $rename_from_text_identifier_length chars" "$rename_from_text_index lineend"]
-+ lappend renamed_filenames [$ctext get "$rename_to_text_index + $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
++
++ set renames [list {}]
++ if {[catch {eval exec git diff $rev --find-renames --stat --raw --diff-filter=R} cmd_result]} {
++ error_popup "[mc "Error getting file rename info for file \"%s\" from commit %s to %s." \
++ $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
++ }
++ set filename [file tail $filepath]
++ set regex_ren {\d+\s\d+\s\S+\s\S+\s\S+\s+(\S+)\s+(\S+)}
++ set regex_ren [subst -nobackslashes -nocommands $regex_ren]
++ if {[regexp -line -- $regex_ren $cmd_result whole_match ren_from ren_to]} {
++ if {$ren_from ne {} && $ren_to ne {}} {
++ lappend renames $ren_from
++ lappend renames $ren_to
+ }
-+ return $renamed_filenames
+ }
++ return $renames
+}
+
proc external_diff {} {
@@ gitk-git/gitk: proc external_diff {} {
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
-+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
++ set renamed_filenames [check_for_renames_in_diff $diffidfrom $diffidto $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
gitk-git/gitk | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 19689765cde..f97904f5fa2 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3775,6 +3775,38 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc check_for_renames_in_diff {diffidfrom diffidto filepath} {
+ global nullid nullid2
+
+ if {$diffidfrom eq $nullid} {
+ set rev [list $diffidto -R]
+ } elseif {$diffidfrom eq $nullid2} {
+ set rev [list $diffidto --cached -R]
+ } elseif {$diffidto eq $nullid} {
+ set rev [list $diffidfrom]
+ } elseif {$diffidto eq $nullid2} {
+ set rev [list $diffidfrom --cached]
+ } else {
+ set rev [list $diffidfrom..$diffidto]
+ }
+
+ set renames [list {}]
+ if {[catch {eval exec git diff $rev --find-renames --stat --raw --diff-filter=R} cmd_result]} {
+ error_popup "[mc "Error getting file rename info for file \"%s\" from commit %s to %s." \
+ $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
+ }
+ set filename [file tail $filepath]
+ set regex_ren {\d+\s\d+\s\S+\s\S+\s\S+\s+(\S+)\s+(\S+)}
+ set regex_ren [subst -nobackslashes -nocommands $regex_ren]
+ if {[regexp -line -- $regex_ren $cmd_result whole_match ren_from ren_to]} {
+ if {$ren_from ne {} && $ren_to ne {}} {
+ lappend renames $ren_from
+ lappend renames $ren_to
+ }
+ }
+ return $renames
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
@@ -3805,8 +3837,16 @@ proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ set renamed_filenames [check_for_renames_in_diff $diffidfrom $diffidto $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
base-commit: 14de3eb34435db79c6e7edc8082c302a26a8330a
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread
* AW: [PATCH v5] gitk: add external diff file rename detection
2025-06-10 8:29 ` [PATCH v5] " ToBoMi via GitGitGadget
@ 2025-06-13 8:18 ` tobias.boesch
2025-06-24 9:05 ` [PATCH v6] " ToBoMi via GitGitGadget
1 sibling, 0 replies; 13+ messages in thread
From: tobias.boesch @ 2025-06-13 8:18 UTC (permalink / raw)
To: ToBoMi via GitGitGadget, git@vger.kernel.org; +Cc: Johannes Sixt
Hi Hannes,
please ignore the V5 patch.
I totally missed to check for the file of interest when checking for renames.
I'll correct that and send an update.
Best wishes
Tobias
> -----Ursprüngliche Nachricht-----
> Von: ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Gesendet: Dienstag, 10. Juni 2025 10:30
> An: git@vger.kernel.org
> Cc: Johannes Sixt <j6t@kdbg.org>; Boesch, Tobias
> <tobias.boesch@miele.com>; Boesch, Tobias <tobias.boesch@miele.com>;
> Boesch, Tobias <tobias.boesch@miele.com>
> Betreff: [PATCH v5] gitk: add external diff file rename detection
>
> From: Tobias Boesch <tobias.boesch@miele.com>
>
> If a file is renamed between commits and an external diff is started through
> gitk on the original or the renamed file name, gitk is unable to open the
> renamed file in the external diff editor.
> It fails to fetch the renamed file from git, because it fetches it using its original
> path in contrast to using the renamed path of the file.
> Detect the rename and open the external diff with the original and the
> renamed file instead of no file (fetch the renamed file path and name from git)
> no matter if the original or the renamed file is selected in gitk.
> Since moved or renamed file are handled the same way do this also for moved
> files.
>
> Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
> ---
> gitk: add external diff file rename detection
>
> Changes since v1:
>
> * Commit message ident
> * Commit message line length
>
> Changes since v2:
>
> * Removed option for rename detection (Adding GUI options seems to be
> not desired - which is understandable)
> * Rebased on current master of git-for-windows
> * Renamed variables for a better understanding
> * Made rename detection also work when the renamed file is selected in
> gitk
>
> Changes since v3:
>
> * Changed message to use present tense, removed bullet points and
> described changes in imperative mood
>
> Changes sine v4:
>
> * Use a git command to gather the changed file paths rather than
> parsing the text from the diff window panel for efficiency and to
> avoid regex containing the filename as a variable.
> * Change != to ne in string comparison
> * removed extra set of parentheses around &&
> * shorter variable names
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-
> 1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-
> 1774/ToBoMi/detect_renamed_files_when_opening_diff-v5
> Pull-Request: https://github.com/gitgitgadget/git/pull/1774
>
> Range-diff vs v4:
>
> 1: 948b94bef5c ! 1: 0d28f189dc3 gitk: add external diff file rename
> detection
> @@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
> "revision $diffid"]
> }
>
> -+proc check_for_renames_in_diff {filepath} {
> -+ global ctext
> ++proc check_for_renames_in_diff {diffidfrom diffidto filepath} {
> ++ global nullid nullid2
> +
> -+ set renamed_filenames [list {}]
> -+ set filename [file tail $filepath]
> -+ set rename_from_text_identifier_length 12
> -+ set rename_to_text_identifier_length 10
> -+ set reg_expr_rename_from {^rename from (.*$filename)}
> -+ set reg_expr_rename_from [subst -nobackslashes -nocommands
> $reg_expr_rename_from]
> -+ set rename_from_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_from 0.0]
> -+ if { ($rename_from_text_index != {})} {
> -+ set reg_expr_rename_to {^rename to (.*)}
> -+ set rename_to_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_to $rename_from_text_index]
> -+ if { ($rename_from_text_index != {}) && ($rename_to_text_index !=
> {}) } {
> -+ lappend renamed_filenames [$ctext get
> "$rename_from_text_index + $rename_from_text_identifier_length chars"
> "$rename_from_text_index lineend"]
> -+ lappend renamed_filenames [$ctext get "$rename_to_text_index +
> $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
> -+ }
> -+ return $renamed_filenames
> ++ if {$diffidfrom eq $nullid} {
> ++ set rev [list $diffidto -R]
> ++ } elseif {$diffidfrom eq $nullid2} {
> ++ set rev [list $diffidto --cached -R]
> ++ } elseif {$diffidto eq $nullid} {
> ++ set rev [list $diffidfrom]
> ++ } elseif {$diffidto eq $nullid2} {
> ++ set rev [list $diffidfrom --cached]
> ++ } else {
> ++ set rev [list $diffidfrom..$diffidto]
> + }
> -+ set reg_expr_rename_to {^rename to (.*$filename)}
> -+ set reg_expr_rename_to [subst -nobackslashes -nocommands
> $reg_expr_rename_to]
> -+ set rename_to_text_index [$ctext search -elide -regexp --
> $reg_expr_rename_to 0.0]
> -+ if { ($rename_to_text_index != {})} {
> -+ set reg_expr_rename_from {^rename from (.*)}
> -+ set rename_from_text_index [$ctext search -backwards -elide -regexp
> -- $reg_expr_rename_from $rename_to_text_index]
> -+ if { ($rename_to_text_index != {}) && ($rename_from_text_index !=
> {}) } {
> -+ lappend renamed_filenames [$ctext get
> "$rename_from_text_index + $rename_from_text_identifier_length chars"
> "$rename_from_text_index lineend"]
> -+ lappend renamed_filenames [$ctext get "$rename_to_text_index +
> $rename_to_text_identifier_length chars" "$rename_to_text_index lineend"]
> ++
> ++ set renames [list {}]
> ++ if {[catch {eval exec git diff $rev --find-renames --stat --raw --diff-
> filter=R} cmd_result]} {
> ++ error_popup "[mc "Error getting file rename info for file \"%s\" from
> commit %s to %s." \
> ++ $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
> ++ }
> ++ set filename [file tail $filepath]
> ++ set regex_ren {\d+\s\d+\s\S+\s\S+\s\S+\s+(\S+)\s+(\S+)}
> ++ set regex_ren [subst -nobackslashes -nocommands $regex_ren]
> ++ if {[regexp -line -- $regex_ren $cmd_result whole_match ren_from
> ren_to]} {
> ++ if {$ren_from ne {} && $ren_to ne {}} {
> ++ lappend renames $ren_from
> ++ lappend renames $ren_to
> + }
> -+ return $renamed_filenames
> + }
> ++ return $renames
> +}
> +
> proc external_diff {} {
> @@ gitk-git/gitk: proc external_diff {} {
> # gather files to diff
> - set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> - set difftofile [external_diff_get_one_file $diffidto $flist_menu_file
> $diffdir]
> -+ set renamed_filenames [check_for_renames_in_diff $flist_menu_file]
> ++ set renamed_filenames [check_for_renames_in_diff $diffidfrom
> $diffidto $flist_menu_file]
> + set rename_from_filename [lindex $renamed_filenames 1]
> + set rename_to_filename [lindex $renamed_filenames 2]
> + if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
>
>
> gitk-git/gitk | 44 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk index 19689765cde..f97904f5fa2
> 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -3775,6 +3775,38 @@ proc external_diff_get_one_file {diffid filename
> diffdir} {
> "revision $diffid"]
> }
>
> +proc check_for_renames_in_diff {diffidfrom diffidto filepath} {
> + global nullid nullid2
> +
> + if {$diffidfrom eq $nullid} {
> + set rev [list $diffidto -R]
> + } elseif {$diffidfrom eq $nullid2} {
> + set rev [list $diffidto --cached -R]
> + } elseif {$diffidto eq $nullid} {
> + set rev [list $diffidfrom]
> + } elseif {$diffidto eq $nullid2} {
> + set rev [list $diffidfrom --cached]
> + } else {
> + set rev [list $diffidfrom..$diffidto]
> + }
> +
> + set renames [list {}]
> + if {[catch {eval exec git diff $rev --find-renames --stat --raw --diff-filter=R}
> cmd_result]} {
> + error_popup "[mc "Error getting file rename info for file \"%s\" from
> commit %s to %s." \
> + $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
> + }
> + set filename [file tail $filepath]
> + set regex_ren {\d+\s\d+\s\S+\s\S+\s\S+\s+(\S+)\s+(\S+)}
> + set regex_ren [subst -nobackslashes -nocommands $regex_ren]
> + if {[regexp -line -- $regex_ren $cmd_result whole_match ren_from ren_to]}
> {
> + if {$ren_from ne {} && $ren_to ne {}} {
> + lappend renames $ren_from
> + lappend renames $ren_to
> + }
> + }
> + return $renames
> +}
> +
> proc external_diff {} {
> global nullid nullid2
> global flist_menu_file
> @@ -3805,8 +3837,16 @@ proc external_diff {} {
> if {$diffdir eq {}} return
>
> # gather files to diff
> - set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> - set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> + set renamed_filenames [check_for_renames_in_diff $diffidfrom $diffidto
> $flist_menu_file]
> + set rename_from_filename [lindex $renamed_filenames 1]
> + set rename_to_filename [lindex $renamed_filenames 2]
> + if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
> + set difffromfile [external_diff_get_one_file $diffidfrom
> $rename_from_filename $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $rename_to_filename
> $diffdir]
> + } else {
> + set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file
> $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $flist_menu_file
> $diffdir]
> + }
>
> if {$difffromfile ne {} && $difftofile ne {}} {
> set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
>
> base-commit: 14de3eb34435db79c6e7edc8082c302a26a8330a
> --
> gitgitgadget
-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v6] gitk: add external diff file rename detection
2025-06-10 8:29 ` [PATCH v5] " ToBoMi via GitGitGadget
2025-06-13 8:18 ` AW: " tobias.boesch
@ 2025-06-24 9:05 ` ToBoMi via GitGitGadget
2025-06-25 6:23 ` Johannes Sixt
1 sibling, 1 reply; 13+ messages in thread
From: ToBoMi via GitGitGadget @ 2025-06-24 9:05 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, tobias.boesch, ToBoMi, Tobias Boesch
From: Tobias Boesch <tobias.boesch@miele.com>
If a file is renamed between commits and an external diff is started
through gitk on the original or the renamed file name,
gitk is unable to open the renamed file in the external diff editor.
It fails to fetch the renamed file from git, because it fetches it
using its original path in contrast to using the renamed path of the
file.
Detect the rename and open the external diff with the original and
the renamed file instead of no file (fetch the renamed file path and
name from git) no matter if the original or the renamed file is
selected in gitk.
Since moved or renamed file are handled the same way do this also
for moved files.
Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
---
gitk: add external diff file rename detection
Changes since v1:
* Commit message ident
* Commit message line length
Changes since v2:
* Removed option for rename detection (Adding GUI options seems to be
not desired - which is understandable)
* Rebased on current master of git-for-windows
* Renamed variables for a better understanding
* Made rename detection also work when the renamed file is selected in
gitk
Changes since v3:
* Changed message to use present tense, removed bullet points and
described changes in imperative mood
Changes sine v4:
* Use a git command to gather the changed file paths rather than
parsing the text from the diff window panel for efficiency and to
avoid regex containing the filename as a variable.
* Change != to ne in string comparison
* removed extra set of parentheses around &&
* shorter variable names
Changes sine v5:
* Include filename in rename check. Find only the file and its renamed
version that is selected in the GUI.
* Escape special characters in the filename to prevent that they are
intepreted as part of a regular expression
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1774%2FToBoMi%2Fdetect_renamed_files_when_opening_diff-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1774/ToBoMi/detect_renamed_files_when_opening_diff-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1774
Range-diff vs v5:
1: 0d28f189dc3 ! 1: f1c71e56324 gitk: add external diff file rename detection
@@ gitk-git/gitk: proc external_diff_get_one_file {diffid filename diffdir} {
+ $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
+ }
+ set filename [file tail $filepath]
-+ set regex_ren {\d+\s\d+\s\S+\s\S+\s\S+\s+(\S+)\s+(\S+)}
-+ set regex_ren [subst -nobackslashes -nocommands $regex_ren]
-+ if {[regexp -line -- $regex_ren $cmd_result whole_match ren_from ren_to]} {
++ set esc_chars {\\ | ? ^ * . $ \[ \] + \( \) \{ \}}
++ foreach char $esc_chars {
++ set filename [string map [list $char \\$char] $filename]
++ }
++ set regex_base {\d+\s\d+\s\S+\s\S+\s\S+\s+}
++ set regex_ren_from $regex_base[subst -nobackslashes -nocommands {(\S+$filename)\s+(\S+)}]
++ set regex_ren_to $regex_base[subst -nobackslashes -nocommands {(\S+)\s+(\S+$filename)}]
++ if {[regexp -line -- $regex_ren_from $cmd_result whole_match ren_from ren_to]} {
++ if {$ren_from ne {} && $ren_to ne {}} {
++ lappend renames $ren_from
++ lappend renames $ren_to
++ }
++ } elseif {[regexp -line -- $regex_ren_to $cmd_result whole_match ren_from ren_to]} {
+ if {$ren_from ne {} && $ren_to ne {}} {
+ lappend renames $ren_from
+ lappend renames $ren_to
gitk-git/gitk | 54 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 2 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 19689765cde..5d4b0fd3a68 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3775,6 +3775,48 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc check_for_renames_in_diff {diffidfrom diffidto filepath} {
+ global nullid nullid2
+
+ if {$diffidfrom eq $nullid} {
+ set rev [list $diffidto -R]
+ } elseif {$diffidfrom eq $nullid2} {
+ set rev [list $diffidto --cached -R]
+ } elseif {$diffidto eq $nullid} {
+ set rev [list $diffidfrom]
+ } elseif {$diffidto eq $nullid2} {
+ set rev [list $diffidfrom --cached]
+ } else {
+ set rev [list $diffidfrom..$diffidto]
+ }
+
+ set renames [list {}]
+ if {[catch {eval exec git diff $rev --find-renames --stat --raw --diff-filter=R} cmd_result]} {
+ error_popup "[mc "Error getting file rename info for file \"%s\" from commit %s to %s." \
+ $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
+ }
+ set filename [file tail $filepath]
+ set esc_chars {\\ | ? ^ * . $ \[ \] + \( \) \{ \}}
+ foreach char $esc_chars {
+ set filename [string map [list $char \\$char] $filename]
+ }
+ set regex_base {\d+\s\d+\s\S+\s\S+\s\S+\s+}
+ set regex_ren_from $regex_base[subst -nobackslashes -nocommands {(\S+$filename)\s+(\S+)}]
+ set regex_ren_to $regex_base[subst -nobackslashes -nocommands {(\S+)\s+(\S+$filename)}]
+ if {[regexp -line -- $regex_ren_from $cmd_result whole_match ren_from ren_to]} {
+ if {$ren_from ne {} && $ren_to ne {}} {
+ lappend renames $ren_from
+ lappend renames $ren_to
+ }
+ } elseif {[regexp -line -- $regex_ren_to $cmd_result whole_match ren_from ren_to]} {
+ if {$ren_from ne {} && $ren_to ne {}} {
+ lappend renames $ren_from
+ lappend renames $ren_to
+ }
+ }
+ return $renames
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
@@ -3805,8 +3847,16 @@ proc external_diff {} {
if {$diffdir eq {}} return
# gather files to diff
- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ set renamed_filenames [check_for_renames_in_diff $diffidfrom $diffidto $flist_menu_file]
+ set rename_from_filename [lindex $renamed_filenames 1]
+ set rename_to_filename [lindex $renamed_filenames 2]
+ if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
+ set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
+ } else {
+ set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
+ set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
+ }
if {$difffromfile ne {} && $difftofile ne {}} {
set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
base-commit: 14de3eb34435db79c6e7edc8082c302a26a8330a
--
gitgitgadget
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v6] gitk: add external diff file rename detection
2025-06-24 9:05 ` [PATCH v6] " ToBoMi via GitGitGadget
@ 2025-06-25 6:23 ` Johannes Sixt
0 siblings, 0 replies; 13+ messages in thread
From: Johannes Sixt @ 2025-06-25 6:23 UTC (permalink / raw)
To: tobias.boesch; +Cc: git, ToBoMi via GitGitGadget
Am 24.06.25 um 11:05 schrieb ToBoMi via GitGitGadget:
> From: Tobias Boesch <tobias.boesch@miele.com>
>
> If a file is renamed between commits and an external diff is started
> through gitk on the original or the renamed file name,
> gitk is unable to open the renamed file in the external diff editor.
> It fails to fetch the renamed file from git, because it fetches it
> using its original path in contrast to using the renamed path of the
> file.
> Detect the rename and open the external diff with the original and
> the renamed file instead of no file (fetch the renamed file path and
> name from git) no matter if the original or the renamed file is
> selected in gitk.
> Since moved or renamed file are handled the same way do this also
> for moved files.
In Git parlance, when we talk about "renamed" files, we always mean
files that have any part of the path name changed (and not just the last
path component). Therefore, this last sentence is redundant.
> Changes sine v4:
>
> * Use a git command to gather the changed file paths rather than
> parsing the text from the diff window panel for efficiency and to
> avoid regex containing the filename as a variable.
An earlier round parsed the rename information from the patch text
panel. I argued that it should not be necessary, because the file list
already knows how to scroll the diff text panel to the correct section
and should already what was renamed. It turns out it's not that simple.
But I still think that this information can be leveraged for this new
purpose and that it is not necessary to invoke an external process. At a
minimum, it should be possible to parse off the rename information from
a smaller section of the patch text, because we know where the section
pertaining to the file of interest starts. Look for uses of the variable
'difffilestart'.
I think that the goal of this patch can be achieved easier by parsing
the patch text panel rather than parsing the output of another `git`
command. I'll still comment on the presented solution just in case it
turns out we must invoke `git` anyway.
> + set renames [list {}]
This constructs a list with one element that is the empty string. I
assume you meant one of these:
set renames [list]
set renames {}
> + if {[catch {eval exec git diff $rev --find-renames --stat --raw --diff-filter=R} cmd_result]} {
A few things I have to note here:
- Don't use porcelain commands, use plumbing commands, i.e., `git
diff-tree`, `git diff-index`, and `git diff-files` instead of `git diff`.
- Place non-option arguments after all options.
- Why use --stat?
- --numstat may be easier to parse than --raw, but see below.
> + error_popup "[mc "Error getting file rename info for file \"%s\" from commit %s to %s." \
> + $filepath $diffidfrom $diffidto] $cmd_result.\n\n"
> + }
> + set filename [file tail $filepath]
> + set esc_chars {\\ | ? ^ * . $ \[ \] + \( \) \{ \}}
> + foreach char $esc_chars {
> + set filename [string map [list $char \\$char] $filename]
> + }
> + set regex_base {\d+\s\d+\s\S+\s\S+\s\S+\s+}
> + set regex_ren_from $regex_base[subst -nobackslashes -nocommands {(\S+$filename)\s+(\S+)}]
This regular expression wants to parse the first of the two file names
that are on the output line. But it assumes that the second of the two
names cannot contain spaces: '(\S+)'. This is not a valid assumption.
Note that the output format of --raw is very restricted. In particular,
the file names are separated from the rest not by space of any kind, but
by TAB. A much stricter regular expression can be used. But still, TAB
is a character that is permitted in file names, and the regular
expression could match still match incorrectly. You can use -z to
separate the parts with a zero byte in an unambiguous way and split the
parts without a regular expression.
> + set regex_ren_to $regex_base[subst -nobackslashes -nocommands {(\S+)\s+(\S+$filename)}]
I don't understand the purpose of 'subst' here. Is this not just
set regex_ren_to $regex_base{(\S+)\s+(\S+}$filename{)}
> @@ -3805,8 +3847,16 @@ proc external_diff {} {
> if {$diffdir eq {}} return
>
> # gather files to diff
> - set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
> - set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> + set renamed_filenames [check_for_renames_in_diff $diffidfrom $diffidto $flist_menu_file]
> + set rename_from_filename [lindex $renamed_filenames 1]
> + set rename_to_filename [lindex $renamed_filenames 2]
> + if { ($rename_from_filename != {}) && ($rename_to_filename != {}) } {
This expression doesn't follow the pattern that we see elsewhere, e.g.,
in the context below.
Please lose the redundant "_filename" in the variable names. In this
context, it's clear that these are names, not indices or somehthing
else. Also, when you look around, you notice that we normally don't use
the underscore in variable names.
> + set difffromfile [external_diff_get_one_file $diffidfrom $rename_from_filename $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $rename_to_filename $diffdir]
> + } else {
> + set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]
> + set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]
> + }
>
> if {$difffromfile ne {} && $difftofile ne {}} {
> set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
-- Hannes
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-25 6:23 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-22 9:27 [PATCH] gitk: added external diff file rename detection ToBoMi via GitGitGadget
2024-09-06 7:28 ` [PATCH v2] " ToBoMi via GitGitGadget
2024-10-02 9:47 ` AW: " tobias.boesch
2025-03-04 13:01 ` [PATCH v3] " ToBoMi via GitGitGadget
2025-03-16 16:21 ` Johannes Sixt
2025-04-28 8:52 ` AW: " tobias.boesch
2025-04-28 8:47 ` [PATCH v4] gitk: add " ToBoMi via GitGitGadget
2025-05-06 19:39 ` Johannes Sixt
2025-06-10 8:28 ` AW: " tobias.boesch
2025-06-10 8:29 ` [PATCH v5] " ToBoMi via GitGitGadget
2025-06-13 8:18 ` AW: " tobias.boesch
2025-06-24 9:05 ` [PATCH v6] " ToBoMi via GitGitGadget
2025-06-25 6:23 ` Johannes Sixt
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 NNTP newsgroup(s).