[PATCH 0/2] gitk: improve handling of submodules in the file list panel

[PATCH 0/2] gitk: improve handling of submodules in the file list panel

[Alex Riesen]

Currently, the submodule entries in the file list panel are mostly ignored.
This series attempts to improve the situation by showing part of submodule
history when focusing it in the file list panel and by adding a menu element
to start gitk in the submodule (similar to git gui).

  [1/2]: gitk: show part of submodule log instead of empty pane when listing trees
  [2/2]: gitk: add an option to run gitk on an item in the file list

 gitk | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 10 deletions(-)

---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

[PATCH 1/2] gitk: show part of submodule log instead of empty pane when listing trees

[Alex Riesen]

From: Alex Riesen <raa.lkml@gmail.com>

Currently, the submodules either are not shown at all (if listing a
committed tree) or a Tcl error appears (when clicking on a submodule
from the index list).

This will make it show first arbitrarily chosen number of commits,
which might be only marginally better.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 gitk | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/gitk b/gitk
index a14d7a1..d34833f 100755
--- a/gitk
+++ b/gitk
@@ -7627,9 +7627,10 @@ proc gettreeline {gtf id} {
 	    if {$i < 0} continue
 	    set fname [string range $line [expr {$i+1}] end]
 	    set line [string range $line 0 [expr {$i-1}]]
-	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
+	    set objtype [lindex $line 1]
+	    if {$diffids ne $nullid2 && $objtype ne "blob" && $objtype ne "commit" } { continue }
 	    set sha1 [lindex $line 2]
-	    lappend treeidlist($id) $sha1
+	    lappend treeidlist($id) "$sha1 $objtype"
 	}
 	if {[string index $fname 0] eq "\""} {
 	    set fname [lindex $fname 0]
@@ -7659,21 +7660,42 @@ proc showfile {f} {
     global ctext_file_names ctext_file_lines
     global ctext commentend
 
+    set submodlog "git\\ log\\ --format='%h\\ %aN:\\ %s'\\ -100"
+    set fcmt ""
     set i [lsearch -exact $treefilelist($diffids) $f]
     if {$i < 0} {
 	puts "oops, $f not in list for id $diffids"
 	return
     }
     if {$diffids eq $nullid} {
-	if {[catch {set bf [open $f r]} err]} {
-	    puts "oops, can't read $f: $err"
-	    return
+	if {[file isdirectory $f]} {
+	    # a submodule
+	    if {[catch {set bf [open "| sh -c cd\\ \"$f\"&&$submodlog" r]} err]} {
+		puts "oops, can't read submodule $f: $err"
+		return
+	    }
+        } else {
+	    if {[catch {set bf [open $f r]} err]} {
+		puts "oops, can't read $f: $err"
+		return
+	    }
 	}
     } else {
-	set blob [lindex $treeidlist($diffids) $i]
-	if {[catch {set bf [open [concat | git cat-file blob $blob] r]} err]} {
-	    puts "oops, error reading blob $blob: $err"
-	    return
+	set bo [lindex $treeidlist($diffids) $i]
+	set blob [lindex $bo 0]
+	set objtype [lindex $bo 1]
+	if { "$objtype" eq "blob" } {
+	    if {[catch {set bf [open [concat | git cat-file blob $blob] r]} err]} {
+		puts "oops, error reading blob $blob: $err"
+		return
+	    }
+	} else {
+	    # also a submodule
+	    if {[catch {set bf [open "| sh -c cd\\ \"$f\"&&$submodlog\\ $blob" r]} err]} {
+		puts "oops, error reading submodule commit: $err"
+		return
+	    }
+	    set fcmt "/"
 	}
     }
     fconfigure $bf -blocking 0 -encoding [get_path_encoding $f]
@@ -7683,7 +7705,7 @@ proc showfile {f} {
     lappend ctext_file_names $f
     lappend ctext_file_lines [lindex [split $commentend "."] 0]
     $ctext insert end "\n"
-    $ctext insert end "$f\n" filesep
+    $ctext insert end "$f$fcmt\n" filesep
     $ctext config -state disabled
     $ctext yview $commentend
     settabs 0
-- 
2.17.0.rc1.56.gb9824190bd


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

Re: [PATCH 1/2] gitk: show part of submodule log instead of empty pane when listing trees

[Stefan Beller]

On Tue, May 8, 2018 at 5:22 AM, Alex Riesen
<alexander.riesen@cetitec.com> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> Currently, the submodules either are not shown at all (if listing a
> committed tree) or a Tcl error appears (when clicking on a submodule
> from the index list).

I do not understand where this appears, yet.
Where do I have to click to see the effects of this patch?

>
> This will make it show first arbitrarily chosen number of commits,
> which might be only marginally better.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
>  gitk | 42 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/gitk b/gitk
> index a14d7a1..d34833f 100755
> --- a/gitk
> +++ b/gitk
> @@ -7627,9 +7627,10 @@ proc gettreeline {gtf id} {
>             if {$i < 0} continue
>             set fname [string range $line [expr {$i+1}] end]
>             set line [string range $line 0 [expr {$i-1}]]
> -           if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
> +           set objtype [lindex $line 1]
> +           if {$diffids ne $nullid2 && $objtype ne "blob" && $objtype ne "commit" } { continue }
>             set sha1 [lindex $line 2]
> -           lappend treeidlist($id) $sha1
> +           lappend treeidlist($id) "$sha1 $objtype"
>         }
>         if {[string index $fname 0] eq "\""} {
>             set fname [lindex $fname 0]
> @@ -7659,21 +7660,42 @@ proc showfile {f} {
>      global ctext_file_names ctext_file_lines
>      global ctext commentend
>
> +    set submodlog "git\\ log\\ --format='%h\\ %aN:\\ %s'\\ -100"

Do we want to respect the config option diff.submodule here?
The -100 is chosen rather arbitrarily. Ideally we'd only walk to the
previous entry?

> +    set fcmt ""
>      set i [lsearch -exact $treefilelist($diffids) $f]
>      if {$i < 0} {
>         puts "oops, $f not in list for id $diffids"
>         return
>      }
>      if {$diffids eq $nullid} {
> -       if {[catch {set bf [open $f r]} err]} {
> -           puts "oops, can't read $f: $err"
> -           return
> +       if {[file isdirectory $f]} {
> +           # a submodule
> +           if {[catch {set bf [open "| sh -c cd\\ \"$f\"&&$submodlog" r]} err]} {

Can we have $submodlog use the "git -C <path> command"
option, then we could save the "cd &&" part, which might even
save us from spawning a shell?

Thanks,
Stefan

Re: [PATCH 1/2] gitk: show part of submodule log instead of empty pane when listing trees

[Alex Riesen]

Stefan Beller, Tue, May 08, 2018 19:07:29 +0200:
> On Tue, May 8, 2018 at 5:22 AM, Alex Riesen
> <alexander.riesen@cetitec.com> wrote:
> > Currently, the submodules either are not shown at all (if listing a
> > committed tree) or a Tcl error appears (when clicking on a submodule
> > from the index list).
> 
> I do not understand where this appears, yet.
> Where do I have to click to see the effects of this patch?

Er. I meant to say the file list panel (bottom right panel). Sorry,
didn't come out clear. I'll reword the commit message next time.

> > @@ -7659,21 +7660,42 @@ proc showfile {f} {
> >      global ctext_file_names ctext_file_lines
> >      global ctext commentend
> >
> > +    set submodlog "git\\ log\\ --format='%h\\ %aN:\\ %s'\\ -100"
> 
> Do we want to respect the config option diff.submodule here?

Probably not. It is already done when the file list panel is in "Patch" mode.
The "Tree" mode of the panel shows the files in full, so the submodules should
be shown similarly: in a format resembling their full (referenced) contents.

> The -100 is chosen rather arbitrarily. Ideally we'd only walk to the
> previous entry?

Yes, the limit is indeed arbitrary. I'm reluctant of listing full history,
though: it might take too long a while (and does, in my line of work). Maybe
an option in the settings? Or some kind of a more natural limit (for 1 second?
Until the end of panel?)

> > -       if {[catch {set bf [open $f r]} err]} {
> > -           puts "oops, can't read $f: $err"
> > -           return
> > +       if {[file isdirectory $f]} {
> > +           # a submodule
> > +           if {[catch {set bf [open "| sh -c cd\\ \"$f\"&&$submodlog" r]} err]} {
> 
> Can we have $submodlog use the "git -C <path> command"
> option, then we could save the "cd &&" part, which might even
> save us from spawning a shell?

That's because I forgot about that option. Of course, I'll fix this.
Also need a shellquote for the path.

Thanks!
Alex

---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

[PATCH 2/2] gitk: add an option to run gitk on an item in the file list

[Alex Riesen]

From: Alex Riesen <raa.lkml@gmail.com>

Similar to a git gui feature which visualizes history in a submodule,
the submodules cause the gitk be started inside the submodule.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 gitk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gitk b/gitk
index d34833f..1ec545e 100755
--- a/gitk
+++ b/gitk
@@ -2682,6 +2682,7 @@ proc makewindow {} {
 	{mc "External diff" command {external_diff}}
 	{mc "Blame parent commit" command {external_blame 1}}
 	{mc "Copy path" command {clipboard clear; clipboard append $flist_menu_file}}
+	{mc "Run gitk on this" command {flist_gitk}}
     }
     $flist_menu configure -tearoff 0
 
@@ -3555,6 +3556,17 @@ proc flist_hl {only} {
     set gdttype [mc "touching paths:"]
 }
 
+proc flist_gitk {} {
+    global flist_menu_file findstring gdttype
+
+    set x [shellquote $flist_menu_file]
+    if {[file isdirectory $flist_menu_file]} {
+	exec sh -c "cd $x&&gitk" &
+    } else {
+	exec gitk -- $x &
+    }
+}
+
 proc gitknewtmpdir {} {
     global diffnum gitktmpdir gitdir env
 
-- 
2.17.0.rc1.56.gb9824190bd


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

Re: [PATCH 2/2] gitk: add an option to run gitk on an item in the file list

[Bert Wesarg]

On Tue, May 8, 2018 at 2:22 PM, Alex Riesen
<alexander.riesen@cetitec.com> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> Similar to a git gui feature which visualizes history in a submodule,
> the submodules cause the gitk be started inside the submodule.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
>  gitk | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/gitk b/gitk
> index d34833f..1ec545e 100755
> --- a/gitk
> +++ b/gitk
> @@ -2682,6 +2682,7 @@ proc makewindow {} {
>         {mc "External diff" command {external_diff}}
>         {mc "Blame parent commit" command {external_blame 1}}
>         {mc "Copy path" command {clipboard clear; clipboard append $flist_menu_file}}
> +       {mc "Run gitk on this" command {flist_gitk}}
>      }
>      $flist_menu configure -tearoff 0
>
> @@ -3555,6 +3556,17 @@ proc flist_hl {only} {
>      set gdttype [mc "touching paths:"]
>  }
>
> +proc flist_gitk {} {
> +    global flist_menu_file findstring gdttype
> +
> +    set x [shellquote $flist_menu_file]

this needs to handle cdup, i.e., if gitk is run from a subdirectory,
all paths needs to be prefixed with $cdup, like: [file join $cdup
$flist_menu_file]

Bert

> +    if {[file isdirectory $flist_menu_file]} {
> +       exec sh -c "cd $x&&gitk" &
> +    } else {
> +       exec gitk -- $x &
> +    }
> +}
> +
>  proc gitknewtmpdir {} {
>      global diffnum gitktmpdir gitdir env
>

Re: [PATCH 2/2] gitk: add an option to run gitk on an item in the file list

[Alex Riesen]

Bert Wesarg, Tue, May 08, 2018 15:17:03 +0200:
> On Tue, May 8, 2018 at 2:22 PM, Alex Riesen <alexander.riesen@cetitec.com> wrote:
> > +proc flist_gitk {} {
> > +    global flist_menu_file findstring gdttype
> > +
> > +    set x [shellquote $flist_menu_file]
> 
> this needs to handle cdup, i.e., if gitk is run from a subdirectory,
> all paths needs to be prefixed with $cdup, like: [file join $cdup
> $flist_menu_file]

That, indeed, is easily done:

    set x [shellquote [file join $cdup $flist_menu_file]]
    if {[file isdirectory $flist_menu_file]} {
	exec sh -c "cd $x&&gitk" &
    } else {
	exec gitk -- $x &
    }


It just doesn't seem to work: gitk does not find any commits!
Maybe that's because the file panel lists only files for the current
subdirectory (without the path from the repo top-level)? E.g.

    mkdir tst
    cd tst
    git init
    mkdir a
    touch top-file a/sub-file
    git add -A ; git commit -m.
    cd a
    gitk

Gitk lists only sub-file.

Frankly, this listing limited to just a sub-directory confuses me a bit. Is
there anyway to get to display full repository without changing to the top
level?


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

Re: [PATCH 2/2] gitk: add an option to run gitk on an item in the file list

[Bert Wesarg]

On Tue, May 8, 2018 at 3:39 PM, Alex Riesen
<alexander.riesen@cetitec.com> wrote:
> Bert Wesarg, Tue, May 08, 2018 15:17:03 +0200:
>> On Tue, May 8, 2018 at 2:22 PM, Alex Riesen <alexander.riesen@cetitec.com> wrote:
>> > +proc flist_gitk {} {
>> > +    global flist_menu_file findstring gdttype
>> > +
>> > +    set x [shellquote $flist_menu_file]
>>
>> this needs to handle cdup, i.e., if gitk is run from a subdirectory,
>> all paths needs to be prefixed with $cdup, like: [file join $cdup
>> $flist_menu_file]
>
> That, indeed, is easily done:
>
>     set x [shellquote [file join $cdup $flist_menu_file]]
>     if {[file isdirectory $flist_menu_file]} {
>         exec sh -c "cd $x&&gitk" &
>     } else {
>         exec gitk -- $x &
>     }
>
>
> It just doesn't seem to work: gitk does not find any commits!
> Maybe that's because the file panel lists only files for the current
> subdirectory (without the path from the repo top-level)? E.g.
>
>     mkdir tst
>     cd tst
>     git init
>     mkdir a
>     touch top-file a/sub-file
>     git add -A ; git commit -m.
>     cd a
>     gitk
>
> Gitk lists only sub-file.
>
> Frankly, this listing limited to just a sub-directory confuses me a bit. Is
> there anyway to get to display full repository without changing to the top
> level?

I noticed that too, while testing your patch and I'm also confused.
But was not able to send a request to Paul yet. ls-tree --full-tree
seems to be one that should be used here, I think.

Bert

[PATCH] gitk: do not limit tree mode listing in the file list panel to current sub-directory

[Alex Riesen]

From: Alex Riesen <raa.lkml@gmail.com>

The previous behavior conflicts with the "Patch" mode of the panel,
which always shows the changes from the top-level of the repository.
It is also impossible to get back to the full listing without restarting
gitk.
---

Bert Wesarg, Wed, May 09, 2018 09:19:55 +0200:
> > Frankly, this listing limited to just a sub-directory confuses me a bit. Is
> > there anyway to get to display full repository without changing to the top
> > level?
> 
> I noticed that too, while testing your patch and I'm also confused.
> But was not able to send a request to Paul yet. ls-tree --full-tree
> seems to be one that should be used here, I think.

Well, I just tried your suggestion. 'ls-files' doesn't have --full-tree, so
for those it is just cd-up.

It is on top of the re-sent series.

 gitk | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gitk b/gitk
index c430dfe..03ead98 100755
--- a/gitk
+++ b/gitk
@@ -7600,18 +7600,18 @@ proc go_to_parent {i} {
 
 proc gettree {id} {
     global treefilelist treeidlist diffids diffmergeid treepending
-    global nullid nullid2
+    global nullid nullid2 cdup
 
     set diffids $id
     unset -nocomplain diffmergeid
     if {![info exists treefilelist($id)]} {
 	if {![info exists treepending]} {
 	    if {$id eq $nullid} {
-		set cmd [list | git ls-files]
+		set cmd [list | git -C $cdup ls-files]
 	    } elseif {$id eq $nullid2} {
-		set cmd [list | git ls-files --stage -t]
+		set cmd [list | git -C $cdup ls-files --stage -t]
 	    } else {
-		set cmd [list | git ls-tree -r $id]
+		set cmd [list | git ls-tree --full-tree -r $id]
 	    }
 	    if {[catch {set gtf [open $cmd r]}]} {
 		return
@@ -7670,7 +7670,7 @@ proc gettreeline {gtf id} {
 proc showfile {f} {
     global treefilelist treeidlist diffids nullid nullid2
     global ctext_file_names ctext_file_lines
-    global ctext commentend
+    global ctext commentend cdup
 
     set submodlog "log --format=%h\\ %aN:\\ %s -100"
     set fcmt ""
@@ -7680,15 +7680,15 @@ proc showfile {f} {
 	return
     }
     if {$diffids eq $nullid} {
-	if {[file isdirectory $f]} {
+	if {[file isdirectory "$cdup$f"]} {
 	    # a submodule
-	    set qf [shellquote $f]
+	    set qf [shellquote "$cdup$f"]
 	    if {[catch {set bf [open "| git -C $qf $submodlog" r]} err]} {
 		puts "oops, can't read submodule $f: $err"
 		return
 	    }
         } else {
-	    if {[catch {set bf [open $f r]} err]} {
+	    if {[catch {set bf [open "$cdup$f" r]} err]} {
 		puts "oops, can't read $f: $err"
 		return
 	    }
@@ -7704,7 +7704,7 @@ proc showfile {f} {
 	    }
 	} else {
 	    # also a submodule
-	    set qf [shellquote $f]
+	    set qf [shellquote "$cdup$f"]
 	    if {[catch {set bf [open "| git -C $qf $submodlog $blob" r]} err]} {
 		puts "oops, error reading submodule commit: $err"
 		return
-- 
2.17.0.593.g2029711e64


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

Re: [PATCH] gitk: do not limit tree mode listing in the file list panel to current sub-directory

[Bert Wesarg]

Thanks.

On Wed, May 9, 2018 at 12:59 PM, Alex Riesen
<alexander.riesen@cetitec.com> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> The previous behavior conflicts with the "Patch" mode of the panel,
> which always shows the changes from the top-level of the repository.
> It is also impossible to get back to the full listing without restarting
> gitk.
> ---
>
> Bert Wesarg, Wed, May 09, 2018 09:19:55 +0200:
>> > Frankly, this listing limited to just a sub-directory confuses me a bit. Is
>> > there anyway to get to display full repository without changing to the top
>> > level?
>>
>> I noticed that too, while testing your patch and I'm also confused.
>> But was not able to send a request to Paul yet. ls-tree --full-tree
>> seems to be one that should be used here, I think.
>
> Well, I just tried your suggestion. 'ls-files' doesn't have --full-tree, so
> for those it is just cd-up.
>
> It is on top of the re-sent series.

I would consider the current behavior as a bug, therefor I would put
this patch first, and than your other fixes/enhancements.

>
>  gitk | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gitk b/gitk
> index c430dfe..03ead98 100755
> --- a/gitk
> +++ b/gitk
> @@ -7600,18 +7600,18 @@ proc go_to_parent {i} {
>
>  proc gettree {id} {
>      global treefilelist treeidlist diffids diffmergeid treepending
> -    global nullid nullid2
> +    global nullid nullid2 cdup
>
>      set diffids $id
>      unset -nocomplain diffmergeid
>      if {![info exists treefilelist($id)]} {
>         if {![info exists treepending]} {
>             if {$id eq $nullid} {
> -               set cmd [list | git ls-files]
> +               set cmd [list | git -C $cdup ls-files]
>             } elseif {$id eq $nullid2} {
> -               set cmd [list | git ls-files --stage -t]
> +               set cmd [list | git -C $cdup ls-files --stage -t]
>             } else {
> -               set cmd [list | git ls-tree -r $id]
> +               set cmd [list | git ls-tree --full-tree -r $id]
>             }
>             if {[catch {set gtf [open $cmd r]}]} {
>                 return
> @@ -7670,7 +7670,7 @@ proc gettreeline {gtf id} {
>  proc showfile {f} {
>      global treefilelist treeidlist diffids nullid nullid2
>      global ctext_file_names ctext_file_lines
> -    global ctext commentend
> +    global ctext commentend cdup
>
>      set submodlog "log --format=%h\\ %aN:\\ %s -100"
>      set fcmt ""
> @@ -7680,15 +7680,15 @@ proc showfile {f} {
>         return
>      }
>      if {$diffids eq $nullid} {
> -       if {[file isdirectory $f]} {
> +       if {[file isdirectory "$cdup$f"]} {
>             # a submodule
> -           set qf [shellquote $f]
> +           set qf [shellquote "$cdup$f"]
>             if {[catch {set bf [open "| git -C $qf $submodlog" r]} err]} {
>                 puts "oops, can't read submodule $f: $err"
>                 return
>             }
>          } else {
> -           if {[catch {set bf [open $f r]} err]} {
> +           if {[catch {set bf [open "$cdup$f" r]} err]} {
>                 puts "oops, can't read $f: $err"
>                 return
>             }
> @@ -7704,7 +7704,7 @@ proc showfile {f} {
>             }
>         } else {
>             # also a submodule
> -           set qf [shellquote $f]
> +           set qf [shellquote "$cdup$f"]
>             if {[catch {set bf [open "| git -C $qf $submodlog $blob" r]} err]} {
>                 puts "oops, error reading submodule commit: $err"
>                 return
> --
> 2.17.0.593.g2029711e64
>
>
> ---
> Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
> https://www.avast.com/antivirus
>

Re: [PATCH] gitk: do not limit tree mode listing in the file list panel to current sub-directory

[Alex Riesen]

Bert Wesarg, Wed, May 09, 2018 14:08:51 +0200:
> >> I noticed that too, while testing your patch and I'm also confused.
> >> But was not able to send a request to Paul yet. ls-tree --full-tree
> >> seems to be one that should be used here, I think.
> >
> > Well, I just tried your suggestion. 'ls-files' doesn't have --full-tree, so
> > for those it is just cd-up.
> >
> > It is on top of the re-sent series.
> 
> I would consider the current behavior as a bug, therefor I would put
> this patch first, and than your other fixes/enhancements.

I might do, just want to hear more opinions on the matter: someone might have
good reasons for the current behaviour and consider a bug the fact that Patch
mode behaves differently, for instance.

And as I completely screwed up the resend of the series, there will definitely
be a re-resend.

Regards,
Alex

---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

[PATCH v2 0/2] gitk: improve handling of submodules in the file list panel

[Alex Riesen]

From: Alex Riesen <raa.lkml@gmail.com>

Currently, the submodule entries in the file list panel are mostly ignored.
This series attempts to improve the situation by showing part of submodule
history when focusing it in the file list panel and by adding a menu element
to start gitk in the submodule (similar to git gui).

This iteration does not address the behaviour of file list panel in tree mode
when gitk is started from a subdirectory (gitk strictly limits the file
listing to the files in that repository, without a way out).
I would like to hear some more opinions regarding changing its behaviour to
always list the full tree.

Alex Riesen (2):
  gitk: show part of submodule log instead of empty pane when listing
    trees
  gitk: add an option to run gitk on an item in the file list

 gitk | 56 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 10 deletions(-)

-- 
2.17.0.593.g2029711e64


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

[PATCH v2 1/2] gitk: show part of submodule log instead of empty pane when listing trees

[Alex Riesen]

From: Alex Riesen <raa.lkml@gmail.com>

Currently, selecting a name in the file list (bottom right) panel in
"Tree" mode does not do anything useful if the name is a submodule.
If gitk is currently showing a commit, the submodule names are not shown
at all (which is very confusing). If the gitk is showing the uncached
change, the submodules are shown, but focusing a submodule name causes a
Tcl error to appear. And finally, if gitk shows the index, the submodule
is presented as its bare name in the diff/file contents panel.

This change will show the first arbitrarily chosen number of commits.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 gitk | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/gitk b/gitk
index a14d7a1..742f36b 100755
--- a/gitk
+++ b/gitk
@@ -7627,9 +7627,10 @@ proc gettreeline {gtf id} {
 	    if {$i < 0} continue
 	    set fname [string range $line [expr {$i+1}] end]
 	    set line [string range $line 0 [expr {$i-1}]]
-	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
+	    set objtype [lindex $line 1]
+	    if {$diffids ne $nullid2 && $objtype ne "blob" && $objtype ne "commit" } { continue }
 	    set sha1 [lindex $line 2]
-	    lappend treeidlist($id) $sha1
+	    lappend treeidlist($id) "$sha1 $objtype"
 	}
 	if {[string index $fname 0] eq "\""} {
 	    set fname [lindex $fname 0]
@@ -7659,21 +7660,44 @@ proc showfile {f} {
     global ctext_file_names ctext_file_lines
     global ctext commentend
 
+    set submodlog "log --format=%h\\ %aN:\\ %s -100"
+    set fcmt ""
     set i [lsearch -exact $treefilelist($diffids) $f]
     if {$i < 0} {
 	puts "oops, $f not in list for id $diffids"
 	return
     }
     if {$diffids eq $nullid} {
-	if {[catch {set bf [open $f r]} err]} {
-	    puts "oops, can't read $f: $err"
-	    return
+	if {[file isdirectory $f]} {
+	    # a submodule
+	    set qf [shellquote $f]
+	    if {[catch {set bf [open "| git -C $qf $submodlog" r]} err]} {
+		puts "oops, can't read submodule $f: $err"
+		return
+	    }
+        } else {
+	    if {[catch {set bf [open $f r]} err]} {
+		puts "oops, can't read $f: $err"
+		return
+	    }
 	}
     } else {
-	set blob [lindex $treeidlist($diffids) $i]
-	if {[catch {set bf [open [concat | git cat-file blob $blob] r]} err]} {
-	    puts "oops, error reading blob $blob: $err"
-	    return
+	set bo [lindex $treeidlist($diffids) $i]
+	set blob [lindex $bo 0]
+	set objtype [lindex $bo 1]
+	if { "$objtype" eq "blob" } {
+	    if {[catch {set bf [open [concat | git cat-file blob $blob] r]} err]} {
+		puts "oops, error reading blob $blob: $err"
+		return
+	    }
+	} else {
+	    # also a submodule
+	    set qf [shellquote $f]
+	    if {[catch {set bf [open "| git -C $qf $submodlog $blob" r]} err]} {
+		puts "oops, error reading submodule commit: $err"
+		return
+	    }
+	    set fcmt "/"
 	}
     }
     fconfigure $bf -blocking 0 -encoding [get_path_encoding $f]
@@ -7683,7 +7707,7 @@ proc showfile {f} {
     lappend ctext_file_names $f
     lappend ctext_file_lines [lindex [split $commentend "."] 0]
     $ctext insert end "\n"
-    $ctext insert end "$f\n" filesep
+    $ctext insert end "$f$fcmt\n" filesep
     $ctext config -state disabled
     $ctext yview $commentend
     settabs 0
-- 
2.17.0.593.g2029711e64


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

[PATCH v2 2/2] gitk: add an option to run gitk on an item in the file list

[Alex Riesen]

From: Alex Riesen <raa.lkml@gmail.com>

Similar to a git gui feature which visualizes history in a submodule,
the submodules cause the gitk be started inside the submodule.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 gitk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gitk b/gitk
index 742f36b..c430dfe 100755
--- a/gitk
+++ b/gitk
@@ -2682,6 +2682,7 @@ proc makewindow {} {
 	{mc "External diff" command {external_diff}}
 	{mc "Blame parent commit" command {external_blame 1}}
 	{mc "Copy path" command {clipboard clear; clipboard append $flist_menu_file}}
+	{mc "Run gitk on this" command {flist_gitk}}
     }
     $flist_menu configure -tearoff 0
 
@@ -3555,6 +3556,17 @@ proc flist_hl {only} {
     set gdttype [mc "touching paths:"]
 }
 
+proc flist_gitk {} {
+    global flist_menu_file findstring gdttype
+
+    set x [shellquote $flist_menu_file]
+    if {[file isdirectory $flist_menu_file]} {
+	exec sh -c "cd $x&&gitk" &
+    } else {
+	exec gitk -- $x &
+    }
+}
+
 proc gitknewtmpdir {} {
     global diffnum gitktmpdir gitdir env
 
-- 
2.17.0.593.g2029711e64


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus

Re: [PATCH v2 0/2] gitk: improve handling of submodules in the file list panel

[Stefan Beller]

On Wed, May 9, 2018 at 5:35 AM, Alex Riesen
<alexander.riesen@cetitec.com> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> Currently, the submodule entries in the file list panel are mostly ignored.
> This series attempts to improve the situation by showing part of submodule
> history when focusing it in the file list panel and by adding a menu element
> to start gitk in the submodule (similar to git gui).
>
> This iteration does not address the behaviour of file list panel in tree mode
> when gitk is started from a subdirectory (gitk strictly limits the file
> listing to the files in that repository, without a way out).
> I would like to hear some more opinions regarding changing its behaviour to
> always list the full tree.
>
> Alex Riesen (2):
>   gitk: show part of submodule log instead of empty pane when listing
>     trees
>   gitk: add an option to run gitk on an item in the file list

both patches look ok, to my untrained eye.