Re: [PATCH 11/18] branch-diff: add tests
Hi Philip, On Fri, 4 May 2018, Philip Oakley wrote: > From: "Johannes Schindelin"> > From: Thomas Rast > > > > These are essentially lifted from https://github.com/trast/tbdiff, with > > light touch-ups to account for the new command name. > > > > Apart from renaming `tbdiff` to `branch-diff`, only one test case needed > > to be adjusted: 11 - 'changed message'. > > > > The underlying reason it had to be adjusted is that diff generation is > > sometimes ambiguous. In this case, a comment line and an empty line are > > added, but it is ambiguous whether they were added after the existing > > empty line, or whether an empty line and the comment line are added > > *before* the existing emtpy line. And apparently xdiff picks a different > > s/emtpy/empty/ Yep, that has been pointed out by others, too... ;-) It is good that this thing gets a really good review *grins* Ciao, Dscho
Re: [PATCH 11/18] branch-diff: add tests
From: "Johannes Schindelin"From: Thomas Rast These are essentially lifted from https://github.com/trast/tbdiff, with light touch-ups to account for the new command name. Apart from renaming `tbdiff` to `branch-diff`, only one test case needed to be adjusted: 11 - 'changed message'. The underlying reason it had to be adjusted is that diff generation is sometimes ambiguous. In this case, a comment line and an empty line are added, but it is ambiguous whether they were added after the existing empty line, or whether an empty line and the comment line are added *before* the existing emtpy line. And apparently xdiff picks a different s/emtpy/empty/ option here than Python's difflib. Signed-off-by: Johannes Schindelin [...] Philip
Re: [PATCH 11/18] branch-diff: add tests
Hi Stefan, On Thu, 3 May 2018, Stefan Beller wrote: > On Thu, May 3, 2018 at 8:30 AM, Johannes Schindelin >wrote: > > From: Thomas Rast > > > > These are essentially lifted from https://github.com/trast/tbdiff, with > > light touch-ups to account for the new command name. > > > > Apart from renaming `tbdiff` to `branch-diff`, only one test case needed > > to be adjusted: 11 - 'changed message'. > > > > The underlying reason it had to be adjusted is that diff generation is > > sometimes ambiguous. In this case, a comment line and an empty line are > > added, but it is ambiguous whether they were added after the existing > > empty line, or whether an empty line and the comment line are added > > *before* the existing emtpy line. And apparently xdiff picks a different > > option here than Python's difflib. > > I think that is the fallout of the diff heuristics. If you are keen on > a 1:1 port, you can disable the diff sliding heuristics and it should > produce the same diff with trailing new lines. I am not keen on a 1:1 port. I am fine with having xdiff generate different diffs than Python's difflib. That's par for the course when relying on a not-quite-well-defined metric. Ciao, Dscho
Re: [PATCH 11/18] branch-diff: add tests
Hi Ævar, On Thu, 3 May 2018, Ævar Arnfjörð Bjarmason wrote: > On Thu, May 03 2018, Johannes Schindelin wrote: > > > *before* the existing emtpy line. And apparently xdiff picks a different > > s/emtpy/empty/ Thanks for the spell check! Dscho
Re: [PATCH 11/18] branch-diff: add tests
On Thu, May 3, 2018 at 8:30 AM, Johannes Schindelinwrote: > From: Thomas Rast > > These are essentially lifted from https://github.com/trast/tbdiff, with > light touch-ups to account for the new command name. > > Apart from renaming `tbdiff` to `branch-diff`, only one test case needed > to be adjusted: 11 - 'changed message'. > > The underlying reason it had to be adjusted is that diff generation is > sometimes ambiguous. In this case, a comment line and an empty line are > added, but it is ambiguous whether they were added after the existing > empty line, or whether an empty line and the comment line are added > *before* the existing emtpy line. And apparently xdiff picks a different > option here than Python's difflib. I think that is the fallout of the diff heuristics. If you are keen on a 1:1 port, you can disable the diff sliding heuristics and it should produce the same diff with trailing new lines.
Re: [PATCH 11/18] branch-diff: add tests
On Thu, May 03 2018, Johannes Schindelin wrote: > *before* the existing emtpy line. And apparently xdiff picks a different s/emtpy/empty/