# [PATCH] gitk: don't highlight submodule diff lines outside submodule diffs 3 messages from 2018-11-06 to 2018-11-06. Participants: Роман Донченко, Stefan Beller. Thread: https://gitlist.dev/t/49774 ## Роман Донченко, 2018-11-06 19:54 Subject: [PATCH] gitk: don't highlight submodule diff lines outside submodule diffs Message-ID: <20181106195417.5456-1-dpb@corrigendum.ru> URL: https://gitlist.dev/e/20181106195417.5456-1-dpb%40corrigendum.ru ``` A line that starts with " <" or " >" is not necessarily a submodule diff line. It might just be a context line in a normal diff, representing a line starting with " <" or " >" respectively. Use the currdiffsubmod variable to track whether we are currently inside a submodule diff and only highlight these lines if we are. Signed-off-by: Роман Донченко --- gitk | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gitk b/gitk index a14d7a1..6bb6dc6 100755 --- a/gitk +++ b/gitk @@ -8109,6 +8109,8 @@ proc parseblobdiffline {ids line} { } # start of a new file set diffinhdr 1 + set currdiffsubmod "" + $ctext insert end "\n" set curdiffstart [$ctext index "end - 1c"] lappend ctext_file_names "" @@ -8191,12 +8193,10 @@ proc parseblobdiffline {ids line} { } else { $ctext insert end "$line\n" filesep } - } elseif {![string compare -length 3 " >" $line]} { - set $currdiffsubmod "" + } elseif {$currdiffsubmod ne "" && ![string compare -length 3 " >" $line]} { set line [encoding convertfrom $diffencoding $line] $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set $currdiffsubmod "" + } elseif {$currdiffsubmod ne "" && ![string compare -length 3 " <" $line]} { set line [encoding convertfrom $diffencoding $line] $ctext insert end "$line\n" d0 } elseif {$diffinhdr} { -- 2.19.1.windows.1 ``` ## Stefan Beller, 2018-11-06 20:06 Subject: Re: [PATCH] gitk: don't highlight submodule diff lines outside submodule diffs Message-ID: URL: https://gitlist.dev/e/CAGZ79kZKN8U5%2BF%2BBAhQMkLuJVupgAvUgCaMt5-e_FDmY1RUY5g%40mail.gmail.com In-Reply-To: <20181106195417.5456-1-dpb@corrigendum.ru> ``` On Tue, Nov 6, 2018 at 12:03 PM Роман Донченко wrote: > > A line that starts with " <" or " >" is not necessarily a submodule > diff line. It might just be a context line in a normal diff, representing > a line starting with " <" or " >" respectively. > > Use the currdiffsubmod variable to track whether we are currently > inside a submodule diff and only highlight these lines if we are. This explanation makes sense, some prior art is at https://public-inbox.org/git/20181021163401.4458-1-dummy@example.com/ which was not taken AFAICT. Thanks, Stefan > > Signed-off-by: Роман Донченко > --- > gitk | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/gitk b/gitk > index a14d7a1..6bb6dc6 100755 > --- a/gitk > +++ b/gitk > @@ -8109,6 +8109,8 @@ proc parseblobdiffline {ids line} { > } > # start of a new file > set diffinhdr 1 > + set currdiffsubmod "" > + > $ctext insert end "\n" > set curdiffstart [$ctext index "end - 1c"] > lappend ctext_file_names "" > @@ -8191,12 +8193,10 @@ proc parseblobdiffline {ids line} { > } else { > $ctext insert end "$line\n" filesep > } > - } elseif {![string compare -length 3 " >" $line]} { > - set $currdiffsubmod "" > + } elseif {$currdiffsubmod ne "" && ![string compare -length 3 " >" $line]} { > set line [encoding convertfrom $diffencoding $line] > $ctext insert end "$line\n" dresult > - } elseif {![string compare -length 3 " <" $line]} { > - set $currdiffsubmod "" > + } elseif {$currdiffsubmod ne "" && ![string compare -length 3 " <" $line]} { > set line [encoding convertfrom $diffencoding $line] > $ctext insert end "$line\n" d0 > } elseif {$diffinhdr} { > -- > 2.19.1.windows.1 > ``` ## Роман Донченко, 2018-11-06 21:56 Subject: Re: [PATCH] gitk: don't highlight submodule diff lines outside submodule diffs Message-ID: URL: https://gitlist.dev/e/fc89fa11-4809-9cb7-93d9-5e9d29c7b82b%40corrigendum.ru In-Reply-To: ``` 06.11.2018 23:06, Stefan Beller пишет: > On Tue, Nov 6, 2018 at 12:03 PM Роман Донченко wrote: >> >> A line that starts with " <" or " >" is not necessarily a submodule >> diff line. It might just be a context line in a normal diff, representing >> a line starting with " <" or " >" respectively. >> >> Use the currdiffsubmod variable to track whether we are currently >> inside a submodule diff and only highlight these lines if we are. > > This explanation makes sense, some prior art is at > https://public-inbox.org/git/20181021163401.4458-1-dummy@example.com/ > which was not taken AFAICT. Didn't see that patch. That said, I think it's incorrect, since it never resets currdiffsubmod back to the empty string, so if a normal diff follows a submodule diff, the same issue will occur. (The `set $currdiffsubmod ""` lines that are already there are effectively useless because they set the variable whose name is the contents of currdiffsubmod, rather than currdiffsubmod itself. I assume it was a typo.) -Roman > > Thanks, > Stefan > >> >> Signed-off-by: Роман Донченко >> --- >> gitk | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/gitk b/gitk >> index a14d7a1..6bb6dc6 100755 >> --- a/gitk >> +++ b/gitk >> @@ -8109,6 +8109,8 @@ proc parseblobdiffline {ids line} { >> } >> # start of a new file >> set diffinhdr 1 >> + set currdiffsubmod "" >> + >> $ctext insert end "\n" >> set curdiffstart [$ctext index "end - 1c"] >> lappend ctext_file_names "" >> @@ -8191,12 +8193,10 @@ proc parseblobdiffline {ids line} { >> } else { >> $ctext insert end "$line\n" filesep >> } >> - } elseif {![string compare -length 3 " >" $line]} { >> - set $currdiffsubmod "" >> + } elseif {$currdiffsubmod ne "" && ![string compare -length 3 " >" $line]} { >> set line [encoding convertfrom $diffencoding $line] >> $ctext insert end "$line\n" dresult >> - } elseif {![string compare -length 3 " <" $line]} { >> - set $currdiffsubmod "" >> + } elseif {$currdiffsubmod ne "" && ![string compare -length 3 " <" $line]} { >> set line [encoding convertfrom $diffencoding $line] >> $ctext insert end "$line\n" d0 >> } elseif {$diffinhdr} { >> -- >> 2.19.1.windows.1 >> ```