Re: [PATCH 5/7] diff.c: add a blocks mode for moved code detection
- From
Stefan Beller <sbeller@google.com>
- Date
- Apr 24, 2018, 22:37 UTC
- Message-ID
- <CAGZ79ka9Tdk=ijbw6Hyxs9OsBMK9O69_3_cY8oX2j=wRc_F5-g@mail.gmail.com>
- In-Reply-To
- <20180424145050.e0ad29d4f304216441656057@google.com>
On Tue, Apr 24, 2018 at 2:50 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
Show 11 quoted lines
> On Tue, 24 Apr 2018 14:03:28 -0700 > Stefan Beller <sbeller@google.com> wrote: > >> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> (https://public-inbox.org/git/87o9j0uljo.fsf@evledraar.gmail.com/) >> Signed-off-by: Stefan Beller <sbeller@google.com> > > Firstly, I don't know if this is the right solution- as written > in the linked e-mail [1], the issue might be more that the config > conflates 2 unrelated things, not that a certain intersection is > missing.
The "plain zebra" or as I call them "blocks", has the "heuristic for a minimum of 20 characters" and "few colors" as its defining features, which solves that use case.
Stepping back a bit, we have different "building blocks" at our disposal: * move detection by line or block * alternating blocks * a heuristic to skip over small chunks (20 alnum chars)
These can be combined independently, so would you expect the user to expect two options for them? For example "--color-moved=zebra" could be split into "--skip-small --alternate-blocks"
Eventually we'll use various colors to inform the user what these building blocks made of the diff.
Ævar wrote:
> Which is what I mean by the current config conflating two (to me) > unrelated things. One is how we, via any method, detect what's moved or > not, and the other is what color/format we use to present this to the > user.
So instead of building blocks we rather want to split into algorithms and presentation layer?
The presentation layer would be things like: * use a different color for moved things * alternate colors for adjacent blocks * paint border of a block (dimmed zebra)
The algorithm side would be * detect moves * detect moves as blocks * skip small heuristic
Am I still missing the big picture?
> [1] https://public-inbox.org/git/87muykuij7.fsf@evledraar.gmail.com/ > > Optional: Probably better to put the link inline, instead of in the > trailer.
ok.
Show 14 quoted lines
> >> -test_expect_success 'detect permutations inside moved code -- dimmed_zebra' ' >> +test_expect_success 'detect blocks of moved code' ' >> git reset --hard && >> cat <<-\EOF >lines.txt && >> long line 1 >> @@ -1271,6 +1271,52 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' ' >> test_config color.diff.newMovedDimmed "normal cyan" && >> test_config color.diff.oldMovedAlternativeDimmed "normal blue" && >> test_config color.diff.newMovedAlternativeDimmed "normal yellow" && > > Add a comment here explaining that these colors do not appear in the > output, but merely set to recognizable values to ensure that they do not > appear in the output.
ok.