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

[Alex Riesen <alexander.riesen@cetitec.com>]

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