Hi Stefan, On Thu, 3 May 2018, Stefan Beller wrote: > >> In addition to that patch, we'd have to buffer commit messages and > >> buffer multiple commits, as that only buffers a diff of a single > >> commit. > > > > ... and make sure that the moved-code logic (which is currently the > > only user of emitted_symbols, correct?) would never be called at the > > same time as we generate the diff. > > The moved detection is all part of the flags of an emitted symbol. > > By design the emitted symbol has easy access to the raw line of the output, > which made it easy for the move detection to work on the lines. AFAICT this > is also desired here as lines are put into a hashmap for comparisons. > (and having it colored differently would make finding the same line > complex using hashmaps) > > I just entertain the thought of having move detection active in a > branch-diff. That would be really cool actually. There are two separate times when we generate a diff in branch-diff: the first time in that `git log -p <range>` call for both ranges, and later, when displaying the changes of old/new commits. (There is actually a third time, when the cost is calculated, but those diffs are not shown, only their line count is used.) It would be relatively easy to use move detection in the diff between old/new commits. But it would be harder to do that with the `git log -p` diffs, as we only color-code them later, not at the time they are generated. In fact, Ævar mentioned that he was pretty happy about the fact that `git branch-diff` accepts all kinds of diff options, when tbdiff emulated only two of them. Ævar mentioned specifically the use of `--color-words`... > >> The benefit would be no invocation of new processes, letting us do > >> more in core. This would allow for tweaking revision walking > >> internally, e.g. passing of options to this command such as rename > >> detection factors, can be passed through easily without the need of > >> translating it back to the command line. > > > > On the other hand, we can simply copy those options to the > > command-line for `log`. Which might even be better, as e.g. `--format` > > changes global state :-( > > ok. I really appreciate the sanity check. It would benefit me on Windows if I could avoid spawning... But I think in this case, it would save me 2*50ms, which is not really worth doing the work for, so far. > Thanks for your patience, And thank you for yours! It *is* important to challenge beliefs during code review, so that the choices are made for the right reasons. Thanks, Dscho
