Daniel Sahlberg wrote on Fri, Aug 13, 2021 at 16:55:38 +0200: > danielsh had two comments on IRC, > https://colabti.org/irclogger/irclogger_log/svn-dev?date=2021-08-09, which > I will try to address here. I'm taking the liberty of copying from the IRC > log to get some context.
Thanks, and sorry for not posting them on the list as I planned to; suffice it to say my Wednesday didn't go according to plan. > Den mån 9 aug. 2021 kl 03:12 skrev Daniel Sahlberg < > daniel.l.sahlb...@gmail.com>: > > I'm proposing to keep the XML tree intact, but filter out the > > information that doesn't match the search pattern. If searching for "add > > 1 file", the result would be as follows: > > > > [[[ > > <logentry revision="12"> > > <logentry revision="11"> > > <logentry revision="10" reverse-merge="false"> > > <author>dsahlberg</author> > > <date>2021-08-08T19:29:10.860725Z</date> > > <msg>add 1 file</msg> > > </logentry> > > </logentry> > > </logentry> > > ]]] > > > > danielsh: 1. Re your "I'm proposing", how would with/without XML compare to > each other? Cf. site/tools/ compiling XML output into plain output. > > (It seems I didn't keep my test repo, but I hope the examples should be > readable anyway). > > For readability, I have reformatted the XML with proper indentation: > [[[ > dsahlberg@daniel-2020:~/temp/wc/c$ ~/svn_trunk_4711/subversion/svn/svn log > --use-merge-history --search 'merge a' > ------------------------------------------------------------------------ > r5 | dsahlberg | 2021-08-09 02:31:34 +0200 (Mon, 09 Aug 2021) | 1 line > Merged via: r6 > > merge a to b > ------------------------------------------------------------------------ > dsahlberg@daniel-2020:~/temp/wc/c$ ~/svn_trunk_4711/subversion/svn/svn log > --use-merge-history --search 'merge a' --xml > <?xml version="1.0" encoding="UTF-8"?> > <log> > <logentry revision="6"> > <logentry reverse-merge="false" revision="5"> > <author>dsahlberg</author> > <date>2021-08-09T00:31:34.969440Z</date> > <msg>merge a to b</msg> > </logentry> > </logentry> > </log> > ]]] > > In plain text output the inheritance is show by the "Merged via:" line, in > XML is found by examining the tree. Okay, and what about the example above with two levels of nesting? How does that compare? Do XML and plain output modes implement filtering the same way (e.g., do both of them elide details of r11)? > I checked site/tools/upcoming.py. It doesn't use "-g" (which is responsible > for generating the recursive tree) so it expects all logentry elements to > be children of <log>, ie, it would not consider revision 5 above. Any tool > that is using "-g" should already be able to traverse a tree with logentry > elements that are children of other logentry elements (which could be > children of other logentry elements which ...). Sure, tools that don't use -g ignore nested <logentry>. No question about that. My question was a bit different, though. In context of your proposal: > > I'm proposing to keep the XML tree intact, but filter out the > > information that doesn't match the search pattern. The proposal does not discuss non-XML output at all. However, the XML and plain output are literally two different output syntaxes of the same subcommand, so I was looking for a comment on whether the proposed change does or does not align with the output of the same command when «--xml» is removed. Specifically, I was pointing out site/tools/upcoming.py as a tool that synthesizes plain output from XML output. Shouldn't any change to XML or plain output retain the possibility to synthesize plain output from XML output? It's not clear to me whether this is the case in trunk@HEAD for «log -g», nor whether it would be the case under the proposal. > > I have run the testsuite with both patch versions without any problems. > > (I have obviously removed XFail() from the test for #4711 in log_tests.py). > > > > danielsh: 2. Wondering about a new svn_stream_t type that wraps another > stream, caches writes until svn_stream_flush(), and has an > undo_last_write() function > danielsh: A 2-tuple struct may well be better :-) > > There is still the need to store "state" somewhere to know if we should > flush() or undo(). Pseudo call-sequence (each line starting with - is a > call to the receiver), I'm using the same WC as above with an additional > revision 7. The calls with r-1 is the invalid revision number used to > generate the </logentry> closing tag after evaluating the merge history > (the -g). > > - receiver r7 > ... match_search_patterns succeed. We want this revision. Output > contents. No children so output the closing tag immediately. Flush(). > - receiver r6 > ... match_search_patterns fail, so we don't want this revision. It has > children so output anyway (but possibly ignoring <msg> etc.). > - receiver r5 (because of -g) > ... match_search_patterns succeed, so we want this revision. Output > contents. No children so output the closing tag immediately. Flush(). > - receiver -1 (this was the end of the merge history) > ... Output closing tag. Now we need to decide if to Flush() or Undo() > depending on if anything was emitted since r6. (The contents of r5 has > already been flush()ed but we also need to flush the </logentry> tag). > The end [riding off into the sunset]. > > The only way I can come up with is to store this information in the > merge_stack. > > That being said, I'm not opposed to use an undo()-able stream but I don't > think it is enough by itself. Thanks. I don't completely follow why we'd need to keep additional state to decide whether to flush() or undo() — I thought (perhaps incorrectly) that the stream's internal queued writes would be all that's needed — but you do, so let's consider this point resolved. Thanks, Daniel. Daniel
