Re: [lmi] Zebras in Git

[Greg Chicares]

On 2018-04-24 14:26, Vadim Zeitlin wrote:
> On Tue, 24 Apr 2018 07:02:35 -0400 (EDT) Greg Chicares <address@hidden> wrote:
> 
> GC> branch: master
> GC> commit 2f7c85099382ba9ef124ba2a358f344ff9dfd42c
> GC> Author: Gregory W. Chicares <address@hidden>
> GC> Commit: Gregory W. Chicares <address@hidden>
> GC> 
> GC>     Transplant implementation of a nested class
> GC>     
> GC>     Forward-declared a nested class, and moved its implementation out of
> GC>     line, to promote readability. This change is merely textual: the
> GC>     nestedness of the inner class is preserved.
> 
>  I'm mostly writing this message because wanted to share my discovery: if
> you're not aware of it yet, as I wasn't myself, since Git 2.15 it's
> possible to view moved lines in a diff in a nicer way. I tried doing "git
> show 2f7c85" first, but this was really unclear, so I redid it as "git show
> -w 2f7c85" which helped a bit as I saw that the code was probably just
> moved, but it was difficult to be sure. So I searched around and found
> about --color-moved option which, ideally still in combination with -w,
> makes the commit perfectly readable using its default "zebra" option
> (unfortunately I can't show it here with all the colours, so you'd have to
> try it yourself to see it). I'm going to use it all the time now when moving
> code around, to verify that I haven't unintentionally changed something in
> the process -- and maybe it can be useful to you too.

Thanks. I too went through every single option in the current git-diff
manual trying to find a set of options that would make this easy. Oddly,
in my zsh history file I cannot find any trace of the options I explored.
I believe I used 'dimmed_zebra', hoping that it would omit the moved
section and "dim" the rest...but the part that I had hoped would be
dimmed was instead highlighted. At that point, I think I fell back on the
old trick of running the same command in two konsole tabs, scrolling them
so that the '+' part in one lined up with the '-' part in the other (that
would be aqua and mauve for the less dimmed among us), and then toggling
back and forth rapidly (hold down Shift, go Left Right Left Right...)
while moving my eyes around the screen until I was satisfied there was no
difference.

I'm still not satisfied: I want git-diff to tell me that the cyan and
fuchsia blocks are identical. Or maybe that's actually what "color-moved"
is supposed to indicate? But...why wasn't "dimmed_zebra" dimmed? Trying
it again, I think maybe the parts that I saw as highlighted really were
dimmed, but my dim white and bright white are so bright that I didn't
perceive a difference between them.

Now I'm all confused. And sleepy. Please tell me--if one '+' block is
cerulean and there's a lilac '-' block of the same size, and those are
the only two blocks in unusual colors...then am I entitled to assume
that git has verified that they're identical?

BTW, did you also notice the '--anchored=' option? In the past, I used
such an option with an ancient GUI diff program for reviewing changes
of this nature. First force the moved parts to line up, and make sure
there's no change in them (which would be highlighted); then un-anchor
the moved parts, and make sure there's no change anywhere else. To me,
the lack of such "anchoring" is the biggest drawback of 'meld', so I
was excited to see that it's been added to git-diff. Or excited to
discover that it's always been there but I hadn't noticed it. Anyway,
it was pretty exciting.

> GC>     Ideally, the inner class's implementation would be moved into the same
> GC>     TU as the outer class; that's not possible because the outer class has
> GC>     a std::vector of the inner class's type. See:
> GC>       http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4371.html
> 
>  However, as long as I'm writing it anyhow, I can't help wondering why
> exactly can't this class be moved in wx_table_generator.cpp?

I would very much like to move it there, but I tried doing exactly that,
and the compiler complained. I interpreted its complaint as meaning that
I couldn't have a std::vector<some_incomplete_type>. That seemed so
twentieth-century a restriction that I searched to see whether they had
relaxed it; the open-std.org quote records what I found, though I found
no indication whether it had been accepted or not. Making the assumption
that it hadn't been accepted, I didn't think to look for its language
changes in my copy of the 2017 standard.

> N4371 only
> seems to require that it should be declared before the container is used
> (possibly indirectly, i.e. from the default ctor), am I missing something?

Either that, or I overlooked something. I might try again after I get
some sleep.

>  BTW, the latest version of this paper, and the one apparently included in
> C++17 seems to be
> 
>       http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4510.html
> 
> (although there doesn't seem to be any important changes since N4371).

Thanks for that update.