Daniel Sahlberg wrote on Fri, 20 Aug 2021 10:30 +00:00: > Den fre 20 aug. 2021 kl 12:11 skrev Daniel Shahaf <d...@daniel.shahaf.name>: > > Daniel Sahlberg wrote on Thu, Aug 19, 2021 at 23:23:49 +0200: > > > [[[ > > ⋮ > > > ------------------------------------------------------------------------ > > > r4 | dsahlberg | 2021-08-19 21:58:28 +0200 (Thu, 19 Aug 2021) | 1 line > > > Merged via: r7, r5 > > > ]]] > > > As can be seen here, it is not visible in the plain text output that r7 > > > was > > > actually a reverse merge. > > > > Hmm. There's an interesting question of whether we can amend the output > > to make that clear. The CLI output is an API, and generally we try not > > to change it unless the user opts in to new features (e.g., when > > changelists were added, the CLI output for users who don't use > > changelists wasn't changed), but there are exceptions (e.g., adding the > > seventh column to `svn status` non-XML output). > > The two options today are: > - Reverse merged via: r7 > or: > - Merged via: r7, r5 > > The code is looking at the the way r4 was merged and then prints the > revision numbers from the merge stack. With the patch we have enough > information on the merge stack to print "(reverse)" on each revision > that was subtractive, for example: > - Merged via: r7 (reverse), r5 > > Another option would be to print both (and each revision only in the > line where it belongs). > - Reverse merged via: r7 > - Merged via: r5 > To do this one would have to loop through the merge_stack twice (at > least) or keep a reasonably long buffer to store the list of revisions > in case of a long stack. >
Hmm. In favour of the second option is that it doesn't require CLI- parsing tools to learn about new syntax, but only to learn about two existing syntaxes being able to appear concurrently. With a little luck, some of them will DTRT. Is there anything the first option can represent that the second one can't? Perhaps some tree-like cases (rN was a merge of both rFOO and rBAR), but those aren't representable in the plain output anyway. For a buffer, we have svn_stringbuf_t that automatically reallocates itself as needed; and in any case, a two-pass algorithm would be easy to implement and to read and performant (it'd still be O(N)). > Both these changes might mess it up for someone consuming the CLI > output and expecting to find one or the other of Merged via: or Reverse > merged via:, or expecting to find just a simple list of revisions. I > can't judge the risk/reward for changing this. Would it be accurate to say that the incumbent output is wrong, misleading, a bug? If so, we could make the change, as a bugfix. The release notes could point it out and point to the XML output as a more-stable alternative. If not, we could record the idea as a milestone=2.0 issue. Cheers, Daniel
