{"thread":{"id":"49774","subject":"[PATCH] gitk: don't highlight submodule diff lines outside submodule diffs","startedAt":"2018-11-06T20:03:13Z","lastAt":"2018-11-06T22:03:59Z","messageCount":3,"participants":["Роман Донченко","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"362606","messageId":"20181106195417.5456-1-dpb@corrigendum.ru","threadId":"49774","inReplyTo":null,"subject":"[PATCH] gitk: don't highlight submodule diff lines outside submodule diffs","fromName":"Роман Донченко","fromEmail":"dpb@corrigendum.ru","sentAt":"2018-11-06T19:54:17Z","receivedAt":"2018-11-06T20:03:13Z","isPatch":true,"sender":{"key":"dpb@corrigendum.ru","avatar":"https://avatars.githubusercontent.com/u/2391761?v=4"},"body":"A line that starts with \"  <\" or \"  >\" is not necessarily a submodule\ndiff line. It might just be a context line in a normal diff, representing\na line starting with \" <\" or \" >\" respectively.\n\nUse the currdiffsubmod variable to track whether we are currently\ninside a submodule diff and only highlight these lines if we are.\n\nSigned-off-by: Роман Донченко <dpb@corrigendum.ru>\n---\n gitk | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex a14d7a1..6bb6dc6 100755\n--- a/gitk\n+++ b/gitk\n@@ -8109,6 +8109,8 @@ proc parseblobdiffline {ids line} {\n \t}\n \t# start of a new file\n \tset diffinhdr 1\n+\tset currdiffsubmod \"\"\n+\n \t$ctext insert end \"\\n\"\n \tset curdiffstart [$ctext index \"end - 1c\"]\n \tlappend ctext_file_names \"\"\n@@ -8191,12 +8193,10 @@ proc parseblobdiffline {ids line} {\n \t} else {\n \t    $ctext insert end \"$line\\n\" filesep\n \t}\n-    } elseif {![string compare -length 3 \"  >\" $line]} {\n-\tset $currdiffsubmod \"\"\n+    } elseif {$currdiffsubmod ne \"\" && ![string compare -length 3 \"  >\" $line]} {\n \tset line [encoding convertfrom $diffencoding $line]\n \t$ctext insert end \"$line\\n\" dresult\n-    } elseif {![string compare -length 3 \"  <\" $line]} {\n-\tset $currdiffsubmod \"\"\n+    } elseif {$currdiffsubmod ne \"\" && ![string compare -length 3 \"  <\" $line]} {\n \tset line [encoding convertfrom $diffencoding $line]\n \t$ctext insert end \"$line\\n\" d0\n     } elseif {$diffinhdr} {\n-- \n2.19.1.windows.1\n\n"},{"id":"362607","messageId":"CAGZ79kZKN8U5+F+BAhQMkLuJVupgAvUgCaMt5-e_FDmY1RUY5g@mail.gmail.com","threadId":"49774","inReplyTo":"20181106195417.5456-1-dpb@corrigendum.ru","subject":"Re: [PATCH] gitk: don't highlight submodule diff lines outside submodule diffs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-06T20:06:49Z","receivedAt":"2018-11-06T20:07:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Nov 6, 2018 at 12:03 PM Роман Донченко <dpb@corrigendum.ru> wrote:\n>\n> A line that starts with \"  <\" or \"  >\" is not necessarily a submodule\n> diff line. It might just be a context line in a normal diff, representing\n> a line starting with \" <\" or \" >\" respectively.\n>\n> Use the currdiffsubmod variable to track whether we are currently\n> inside a submodule diff and only highlight these lines if we are.\n\nThis explanation makes sense, some prior art is at\nhttps://public-inbox.org/git/20181021163401.4458-1-dummy@example.com/\nwhich was not taken AFAICT.\n\nThanks,\nStefan\n\n>\n> Signed-off-by: Роман Донченко <dpb@corrigendum.ru>\n> ---\n>  gitk | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/gitk b/gitk\n> index a14d7a1..6bb6dc6 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -8109,6 +8109,8 @@ proc parseblobdiffline {ids line} {\n>         }\n>         # start of a new file\n>         set diffinhdr 1\n> +       set currdiffsubmod \"\"\n> +\n>         $ctext insert end \"\\n\"\n>         set curdiffstart [$ctext index \"end - 1c\"]\n>         lappend ctext_file_names \"\"\n> @@ -8191,12 +8193,10 @@ proc parseblobdiffline {ids line} {\n>         } else {\n>             $ctext insert end \"$line\\n\" filesep\n>         }\n> -    } elseif {![string compare -length 3 \"  >\" $line]} {\n> -       set $currdiffsubmod \"\"\n> +    } elseif {$currdiffsubmod ne \"\" && ![string compare -length 3 \"  >\" $line]} {\n>         set line [encoding convertfrom $diffencoding $line]\n>         $ctext insert end \"$line\\n\" dresult\n> -    } elseif {![string compare -length 3 \"  <\" $line]} {\n> -       set $currdiffsubmod \"\"\n> +    } elseif {$currdiffsubmod ne \"\" && ![string compare -length 3 \"  <\" $line]} {\n>         set line [encoding convertfrom $diffencoding $line]\n>         $ctext insert end \"$line\\n\" d0\n>      } elseif {$diffinhdr} {\n> --\n> 2.19.1.windows.1\n>\n"},{"id":"362616","messageId":"fc89fa11-4809-9cb7-93d9-5e9d29c7b82b@corrigendum.ru","threadId":"49774","inReplyTo":"CAGZ79kZKN8U5+F+BAhQMkLuJVupgAvUgCaMt5-e_FDmY1RUY5g@mail.gmail.com","subject":"Re: [PATCH] gitk: don't highlight submodule diff lines outside submodule diffs","fromName":"Роман Донченко","fromEmail":"dpb@corrigendum.ru","sentAt":"2018-11-06T21:56:40Z","receivedAt":"2018-11-06T22:03:59Z","isPatch":true,"sender":{"key":"dpb@corrigendum.ru","avatar":"https://avatars.githubusercontent.com/u/2391761?v=4"},"body":"06.11.2018 23:06, Stefan Beller пишет:\n> On Tue, Nov 6, 2018 at 12:03 PM Роман Донченко <dpb@corrigendum.ru> wrote:\n>>\n>> A line that starts with \"  <\" or \"  >\" is not necessarily a submodule\n>> diff line. It might just be a context line in a normal diff, representing\n>> a line starting with \" <\" or \" >\" respectively.\n>>\n>> Use the currdiffsubmod variable to track whether we are currently\n>> inside a submodule diff and only highlight these lines if we are.\n> \n> This explanation makes sense, some prior art is at\n> https://public-inbox.org/git/20181021163401.4458-1-dummy@example.com/\n> which was not taken AFAICT.\n\nDidn't see that patch. That said, I think it's incorrect, since it never \nresets currdiffsubmod back to the empty string, so if a normal diff \nfollows a submodule diff, the same issue will occur.\n\n(The `set $currdiffsubmod \"\"` lines that are already there are \neffectively useless because they set the variable whose name is the \ncontents of currdiffsubmod, rather than currdiffsubmod itself. I assume\nit was a typo.)\n\n-Roman\n\n> \n> Thanks,\n> Stefan\n> \n>>\n>> Signed-off-by: Роман Донченко <dpb@corrigendum.ru>\n>> ---\n>>   gitk | 8 ++++----\n>>   1 file changed, 4 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/gitk b/gitk\n>> index a14d7a1..6bb6dc6 100755\n>> --- a/gitk\n>> +++ b/gitk\n>> @@ -8109,6 +8109,8 @@ proc parseblobdiffline {ids line} {\n>>          }\n>>          # start of a new file\n>>          set diffinhdr 1\n>> +       set currdiffsubmod \"\"\n>> +\n>>          $ctext insert end \"\\n\"\n>>          set curdiffstart [$ctext index \"end - 1c\"]\n>>          lappend ctext_file_names \"\"\n>> @@ -8191,12 +8193,10 @@ proc parseblobdiffline {ids line} {\n>>          } else {\n>>              $ctext insert end \"$line\\n\" filesep\n>>          }\n>> -    } elseif {![string compare -length 3 \"  >\" $line]} {\n>> -       set $currdiffsubmod \"\"\n>> +    } elseif {$currdiffsubmod ne \"\" && ![string compare -length 3 \"  >\" $line]} {\n>>          set line [encoding convertfrom $diffencoding $line]\n>>          $ctext insert end \"$line\\n\" dresult\n>> -    } elseif {![string compare -length 3 \"  <\" $line]} {\n>> -       set $currdiffsubmod \"\"\n>> +    } elseif {$currdiffsubmod ne \"\" && ![string compare -length 3 \"  <\" $line]} {\n>>          set line [encoding convertfrom $diffencoding $line]\n>>          $ctext insert end \"$line\\n\" d0\n>>       } elseif {$diffinhdr} {\n>> --\n>> 2.19.1.windows.1\n>>\n"}]}