Karl Fogel wrote on Wed, Mar 30, 2022 at 17:58:55 -0500:
> On 30 Mar 2022, Julian Foad wrote:
> > Karl Fogel wrote:
> > > I think printing these messages to stderr makes the most sense.
> > > There are plenty of programs out there that parse the stdout of
> > > 'svn'; we don't want to interfere with them.
> > > 
> > > As you point out, it's especially important for 'svn diff' and 'svn
> > > cat' that stdout remain untainted.  Therefore, we can either always
> > > print these messages to stderr (across all commands), or not print
> > > them for 'svn diff' and 'svn cat' (but that would be an odd
> > > inconsistency).
> > > 
> > > > Anybody want to recommend what we should do for 'cat' and
> > > > 'diff'?
> > > 
> > > As per above: I think we should print the messages to stderr for
> > > everything, including those two.
> > 
> > Printing progress notifications for data-output commands (diff, cat) to
> > stderr does however invite bikeshedding. Currently in our test suite we
> > assume any stderr output (from diff, cat) should be flagged as a test
> > failure. We can change that, but it indicates that some users may
> > consider it a failure too. We would need to agree and decide whether
> > that's going to be unconditional or if the user needs to be able to turn
> > it off for convenience and for backward compatibility.
> > 
> > Because this could be dragged out I'm filing it as a lower priority for
> > now. We can get back to it. (If someone feels able to resolve it,
> > great.)
> 
> Good points, and +1 to prioritizing it lower down relative to shipping the
> main thing!

I'm a bit hesitant about disabling notifications _entirely_ in cat-cmd.c
and diff-cmd.c.  Disabling all notifications (as opposed to only
hydration-related notifications which we focus on right now) seems like
it could easily have unintended consequences.  Do we do that elsewhere?
E.g., in --xml mode?

«diff» isn't unique in having parseable output.  On the contrary, all
our subcommands have parseable output, whether it's "unidiffs" or "seven
columns, then a column of spaces, then filenames" or "XML in <this>
schema" or "RFC822-esque data with localized field names".  Sure, «svn
diff»'s output may be consumed by other tools, but that also goes for
other subcommands (e.g., help/info/ls/proplist/status all have
machine-parseable output even in non-«--xml» mode).  So, I am leaning to
the opinion that «svn diff» isn't a special case, and «diff» and those
other commands should all use the same rules for their output.

What are those rules?  Good question.  stdout isn't a good place for the
hydration notifications, since those are of interest to the (human)
producer of the output but not to the consumer of the output; stderr
isn't a good place, since CLI consumers may interpret that as an error
indication (though the exit code would be zero, EXIT_SUCCESS); and
/dev/null isn't a good place either, since we added those notifications
for a reason.

«svn cat», on the other hand, _is_ unique, in that it's the only command
whose output format is not controlled by us.  That means we can't emit
anything to stdout in «svn cat» without possibly mangling user data.
So, for «svn cat» we don't have the option of emitting notifications to
stdout; we have only the other options discussed above.

Cheers,

Daniel

Reply via email to