You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@apache.org> on 2019/11/06 10:36:10 UTC

svn status should not show unmodified files in changelists

svn status doesn't generally show unmodified files, except with '-v'.
However, in changelists, it shows unmodified files, even without '-v'

I am finding this to be a inconsistent, and a nuisance because I am 
currently working on a task in which I have a lot of files potentially 
modified, in two or more groups which I want to commit separately, 
scattered across paths. The use case seems right for changelists, but 
"svn status" shows a screenful of non-changed files.

Demo:
[[[
$ touch a b c d; svn add a b c d; svn ci -m ""; svn up
...
$ echo hello > b
$ echo hello > d
$ svn cl foo c d
A [foo] c
A [foo] d
$ svn status -v
                  1        1 julianfoad   .
                  1        1 julianfoad   a
M                1        1 julianfoad   b
--- Changelist 'foo':
                  1        1 julianfoad   c
M                1        1 julianfoad   d
$ svn status
M       b
--- Changelist 'foo':
         c
M       d
]]]

File 'c' shows up in the plain 'svn status' even though it is unmodified.

I propose we should change this so unmodified files are not listed. It 
would look like this:

[[[
$ svn status
M       b
--- Changelist 'foo':
M       d
]]]

A changelist heading is printed only when a file in that changelist is 
listed, so there will be no heading if no files in the changelist are 
modified. It would look like this:

[[[
$ svn revert b d
Reverted 'b'
Reverted 'd'
$ svn status -v
                  1        1 julianfoad   .
                  1        1 julianfoad   a
                  1        1 julianfoad   b
--- Changelist 'foo':
                  1        1 julianfoad   c
                  1        1 julianfoad   d
$ svn status
]]]

Thoughts?

- Julian

Re: svn status should not show unmodified files in changelists

Posted by Branko Čibej <br...@apache.org>.
On 08.11.2019 10:48, Daniel Shahaf wrote:
> Nathan Hartman wrote on Fri, Nov 08, 2019 at 00:58:29 -0500:
>> In my mind, listing changelists (and which files are "in" them) should
>> be a documented part of the changelist feature, not something achieved
>> by a non-obvious invocation of 'svn status'.
>>
>> 'svn changelist --list' is not congruent to the way properties and
>> shelves are listed; i.e., those have a separate top-level command like
>> 'svn proplist'. So, perhaps 'svn changelists' (plural)? That lines up
>> with 'svn x-shelves', but I don't like that it differs only by one
>> letter from 'svn changelist'.
> Bikeshed, but: «svn changelist-list».
>
> That's analogous to both «proplist» and «x-shelf-list».


We could (should?) introduce subcommands. For example:

  * 'svn proplist' would become shorthand for the canonical 'svn prop list'
  * 'svn changelist' would become shorthand for the canonical 'svn
    changelist add'
  * new commands would have this structure from the start (e.g., 'svn
    x-shelf list')

and so on. It would need some support in the parser and some design
decisions (e.g., can you put options between the command and its
subcommand? can different subcommands accept different options? etc.).

In any case something like this would let us structure the commands more
logically whilst also keeping backward compatibility.

-- Brane


Re: svn status should not show unmodified files in changelists

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> The intended reading of the example is [...]

I know, but we're not going to get much further by focusing on that example.

> I wonder if we should invent some notion of "quiet" changelist, [...]

A quality of being "quiet in the context of svn-status" is not a good 
quality to build in to a notion of changelists.

If there are going to be different kinds of changelists, the distinction 
needs to be something more fundamental, more real-world-useful, than that.

> Furtheromre, I wonder if we should allow to configure changelists via
> properties, somehow.

Yes, perhaps, something along the lines of how auto-props can be 
configured.  In fact, this reminds me that changelist assignment is in 
the nature of a wc-only property (like svn:wc:*, svn:entry:*).  The WC 
design could probably be simplified by storing it as such.

- Julian

Re: svn status should not show unmodified files in changelists

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Fri, 08 Nov 2019 13:00 +00:00:
> Nathan Hartman wrote:
> >  > Friday% svn st -q
> >  > --- Changelist 'foo':
> >  > M       iota
> >  > Friday% vi A/mu
> >  > Friday% svn commit -mm --cl foo
> >  > Sending        iota
> >  > Sending        A/mu
> >  > # ... "Revert accidental commit"
> > 
> > You're right. That would be immensely irritating...
> 
> The opposite reading of that example is,
> # "Oh!  I'm glad I used a changelist.  I had forgotten both files were 
> in my changelist and I would have accidentally omitted one from the 
> commit if I had specified a filename instead of a changelist."

I agree that changelists can have a gladdening effect in some cases, but
that example isn't one of them.  In that example, the user had to revert
an accidental commit.  He hadn't been gladdened.

The intended reading of the example is that after exiting vi, the user
reviewed the output in the terminal, assumed that A/mu wasn't in any
changelist, wanted to commit iota, and was surprised by the fact that
'commit --cl foo' committed not only iota but also A/mu.

> But that example shows a trivial use of changelists -- just one or two 
> files, one commit.

The example output showed just 'iota' because it was a minimal example.
There might have been any number of files in the 'foo' changelist and
modified, and the user wanted to commit all of them but not A/mu.

> In my example, I have about 60 files in one changelist and 20 in each of 
> three more changelists, and it makes sense to keep these changelist 
> assignments over a series of commits over many weeks.  In each commit I 
> only touch one or a few files.
> 
> That changes things.  I frequently want to check what files I have 
> changed.  I don't need or want to keep seeing my changes interspersed in 
> a screenful of blank output.  I don't need constant reminding of the 
> changelist assignments of all my hundred-and-twenty files when I'm just 
> looking to see what's ready for commit.

It certainly does change things.  You describe a use-case in which
changelists are effectively permanent fixtures of a working copy, or
possibly even of several related working copies (e.g., different
branches of a single project). That's an interesting use-case, but I don't
think it was envisioned when changelists were introduced, so it's no
surprise that there's some impedance mismatch between the implemented
and desired functionalities.

Thinking out loud:

I wonder if we should invent some notion of "quiet" changelist, i.e.,
a changelist whose files aren't shown in 'status' output if they're
unmodified.  Effectively, we'd have one bit of metadata for each
changelist; changelists that have the bit set would have the behaviour
Julian proposed; and other changelists would continue to have the current
behaviour.  (I deliberately leave unspecified for the time being the
question of how we'd set, unset, and query that bit.)  The output could
look like this:

[[[
# Default: loud changelist
% svn st
--- Changelist 'foo':
        lorem
M       ipsum
% 
⋮
# New behaviour: silent changelist
% svn st
--- Silent changelist 'foo' (42 files not shown):
M       ipsum
% 
]]]

Furtheromre, I wonder if we should allow to configure changelists via
properties, somehow.  For example, we might have «svn ps svn:changelists
"foo: subversion/svn/svn.c subversion/svnserve/svnserve.c …
subversion/svnsync/svnsync.c" ./» and then do «svn st --cl :foo» to
query the cmdline binaries' entry point C files.

Daniel

Re: svn status should not show unmodified files in changelists

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Nov 8, 2019 at 8:00 AM Julian Foad <ju...@apache.org> wrote:

> Nathan Hartman wrote:
> >  > Friday% svn st -q
> >  > --- Changelist 'foo':
> >  > M       iota
> >  > Friday% vi A/mu
> >  > Friday% svn commit -mm --cl foo
> >  > Sending        iota
> >  > Sending        A/mu
> >  > # ... "Revert accidental commit"
> >
> > You're right. That would be immensely irritating...
>
> The opposite reading of that example is,
> # "Oh!  I'm glad I used a changelist.  I had forgotten both files were
> in my changelist and I would have accidentally omitted one from the
> commit if I had specified a filename instead of a changelist."
>
> But that example shows a trivial use of changelists -- just one or two
> files, one commit.
>
> In my example, I have about 60 files in one changelist and 20 in each of
> three more changelists, and it makes sense to keep these changelist
> assignments over a series of commits over many weeks.  In each commit I
> only touch one or a few files.
>
> That changes things.  I frequently want to check what files I have
> changed.  I don't need or want to keep seeing my changes interspersed in
> a screenful of blank output.  I don't need constant reminding of the
> changelist assignments of all my hundred-and-twenty files when I'm just
> looking to see what's ready for commit.


That is a good point. I hadn't properly considered the case of
changelists with more than a handful of items in them. But you're
right, nothing stops someone from carefully constructing changelists
with many hundreds of items in them. Which means that it would be
"immensely irritating" (to borrow my own earlier words) if you forget
to type the '--keep-changelists' argument to 'svn commit'.

So, if we revamp/reinvent/reboot the changelist feature, perhaps with
the addition of Daniel's suggestion to save changelists via a
svn:changelists property, perhaps also the ability to associate one
file to multiple changelists, then I'd lean toward my earlier
statement that 'svn status' without other options should print
modified items only, even if they're in a changelist; and that
printing the list of changelists and their contents (modified or not)
should be a 'svn changelists' or 'svn changelist-list' or 'svn
changelist --list' or some such command.

Nathan

Re: svn status should not show unmodified files in changelists

Posted by Julian Foad <ju...@apache.org>.
Nathan Hartman wrote:
>  > Friday% svn st -q
>  > --- Changelist 'foo':
>  > M       iota
>  > Friday% vi A/mu
>  > Friday% svn commit -mm --cl foo
>  > Sending        iota
>  > Sending        A/mu
>  > # ... "Revert accidental commit"
> 
> You're right. That would be immensely irritating...

The opposite reading of that example is,
# "Oh!  I'm glad I used a changelist.  I had forgotten both files were 
in my changelist and I would have accidentally omitted one from the 
commit if I had specified a filename instead of a changelist."

But that example shows a trivial use of changelists -- just one or two 
files, one commit.

In my example, I have about 60 files in one changelist and 20 in each of 
three more changelists, and it makes sense to keep these changelist 
assignments over a series of commits over many weeks.  In each commit I 
only touch one or a few files.

That changes things.  I frequently want to check what files I have 
changed.  I don't need or want to keep seeing my changes interspersed in 
a screenful of blank output.  I don't need constant reminding of the 
changelist assignments of all my hundred-and-twenty files when I'm just 
looking to see what's ready for commit.

> Is it possible that printing unmodified items in a changelist was a
> deliberate design decision way back when the changelist features came
> to light, and we simply forgot?

Quite possible; and we haven't necessarily forgotten, just re-visiting it.

> We could say that 'svn status' by default only prints files that are
> "interesting" (for some definition of "interesting") and the virtue of
> being associated to a changelist makes a file "interesting" whether
> it's modified or not. ???

It seems only to make sense in trivial uses of changelists, and not when 
tracking lots of files over many commits.

- Julian

Re: svn status should not show unmodified files in changelists

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Nov 8, 2019 at 4:48 AM Daniel Shahaf <d....@daniel.shahaf.name>
wrote:
Nathan Hartman wrote on Fri, Nov 08, 2019 at 00:58:29 -0500:
>> Daniel suggested to leave the behavior of 'svn status' as-is, unless
>> 'svn status' is run with both -v and -q.
>
> That's not what I suggested.

I misunderstood... sorry! More below...

>> I think this is a bug. 'svn status' does not print unmodified items in
>> the "default" changelist, and that is as it should be. For
>> consistency, unmodified items in an explicit changelist shouldn't be
>> printed either.
>
> What bothers me is the following use-case:
>
> [[[
> Monday% svn revert -q iota A/mu
> Monday% svn cl foo iota A/mu
> ⋮
> Friday% svn st -q
> --- Changelist 'foo':
> M       iota
> Friday% vi A/mu
> Friday% svn commit -mm --cl foo
> Sending        iota
> Sending        A/mu
> Transmitting file data ..done
> Committing transaction...
> Committed revision 2.
> Friday% svn up -q
> Friday% svn merge -c -2 A/mu
> Friday% svn ci -m "Revert accidental commit"
> ]]]

You're right. That would be immensely irritating...

Is it possible that printing unmodified items in a changelist was a
deliberate design decision way back when the changelist features came
to light, and we simply forgot?

We could say that 'svn status' by default only prints files that are
"interesting" (for some definition of "interesting") and the virtue of
being associated to a changelist makes a file "interesting" whether
it's modified or not. ???

>> 'svn changelist --list' is not congruent to the way properties and
>> shelves are listed; i.e., those have a separate top-level command like
>> 'svn proplist'. So, perhaps 'svn changelists' (plural)? That lines up
>> with 'svn x-shelves', but I don't like that it differs only by one
>> letter from 'svn changelist'.
>
> Bikeshed, but: «svn changelist-list».
>
> That's analogous to both «proplist» and «x-shelf-list».

This is a completely different discussion but I think top-level
commands would be better reserved for: basic operations, namespaces
for other features, and shortcuts for power users.

Basic operations are things like checkout, add, delete, move, copy,
list, status, info, log, revert, update, commit, switch, merge,
resolve, etc.

Namespaces organize related non-basic operations together. For
example, instead of 5 commands for dealing with properties (propdel,
propedit, propget, proplist, propset), under such a system these would
be subcommands of a single {property,properties,prop,props} top-level
command:

$ svn properties list
$ svn prop ls

Shortcuts for power users would be kept as top-level commands so that
power users won't have to type too much:

$ svn pl

is the "power user" version of "svn properties list."

For backwards compatibility, we'd have to keep all of the top-level
long names that we already have, too.

Then, a revamped changelists feature could be completely managed as
subcommands of a single {changelist,changelists,cl} top-level
command to list, create, and delete changelists, and add / remove
items to changelists. The same would be true of all new features
that need a suite of their own commands...

Since we're on the subject of bikesheds ;-)

Nathan

Re: svn status should not show unmodified files in changelists

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Fri, Nov 08, 2019 at 00:58:29 -0500:
> Daniel suggested to leave the behavior of 'svn status' as-is, unless
> 'svn status' is run with both -v and -q.

That's not what I suggested.

I suggested to modify the behaviour if -q is passed, *regardless* of whether -v
is passed or not.  In the truth table diff I posted, _two_ rows were modified,
not just one.

> I'm not really sure how this morphed into a discussion about 'svn
> info', tab-delimited values, RFC822, XML, and feature branches,

It started with my parenthetical observation that «svn info»'s output is hard
to parse in a shell script because it's not line based.  (Example: list all the
directories that are at a depth other than 'infinity'.)

[I solve this with «perl -nE 'BEGIN { $/ = "" }' -E '…'», but that's a mouthful
to type, and doesn't even have the output parsed to a hash.  With JSON output
it'd be more elegant: «svn info --json | jq 'map(select(.depth) | .path)' | jq -r '.[]'».]

> but if I may back up for a moment and talk about changelists again...

Of course you may.  The subject line was changed; the two discussions can
continue in parallel.

> I think this is a bug. 'svn status' does not print unmodified items in
> the "default" changelist, and that is as it should be. For
> consistency, unmodified items in an explicit changelist shouldn't be
> printed either.

What bothers me is the following use-case:

[[[
Monday% svn revert -q iota A/mu
Monday% svn cl foo iota A/mu
⋮
Friday% svn st -q
--- Changelist 'foo':
M       iota
Friday% vi A/mu
Friday% svn commit -mm --cl foo
Sending        iota
Sending        A/mu
Transmitting file data ..done
Committing transaction...
Committed revision 2.
Friday% svn up -q
Friday% svn merge -c -2 A/mu
Friday% svn ci -m "Revert accidental commit"
]]]

That can't happen in 1.14 because 'st -q' would list A/mu, but it would be
possible under all of the proposals so far (including mine).

What happened in this example is that A/mu was in the 'foo' changelist, but
that wasn't shown in the output.  Effectively, A/mu's being in a changelist was
hidden state, something svn knew about but the user had to remember.  We
shouldn't have hidden states; the output should show all relevant information.
(Of course, the million dollar question is what information is relevant…)

I'm not sure what to do.  The first ideas that come to mind are:
.
    (1) keep the status quo
    (2) only hide unmodified, changelisted files if -q is passed twice or more
    (3) invent a new option flag for hiding unmodified, changelisted files
    (4) declare this example a non-problem
.
but there's probably a better solution.

> In my mind, listing changelists (and which files are "in" them) should
> be a documented part of the changelist feature, not something achieved
> by a non-obvious invocation of 'svn status'.
> 
> 'svn changelist --list' is not congruent to the way properties and
> shelves are listed; i.e., those have a separate top-level command like
> 'svn proplist'. So, perhaps 'svn changelists' (plural)? That lines up
> with 'svn x-shelves', but I don't like that it differs only by one
> letter from 'svn changelist'.

Bikeshed, but: «svn changelist-list».

That's analogous to both «proplist» and «x-shelf-list».

> Perhaps 'svn changelist' or 'svn changelists' with no additional parameters
> should print the changelists and their files instead of the current behavior
> which outputs E205001?

Hmm…

Cheers,

Daniel

Re: svn status should not show unmodified files in changelists

Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Nov 7, 2019 at 6:13 AM Daniel Shahaf <d....@daniel.shahaf.name> wrote:

> 2. As Nathan said, this can make it difficult to answer "Which unmodified
> files
> are in a changelist?".  (Yes, it's still possible to answer this by parsing
> the output of «svn info», but that's far from elegant, particularly since
> it's
> not line-based.)


Much has happened, so a brief recap:

Julian points out that 'svn status' prints unmodified items if they
are in a changelist, which is contrary to the definition of 'svn
status' (without modifiers) printing only locally modified items.

I pointed out that if this is fixed, we need a way to list changelists
and what files are associated to them.

You can use 'svn status -v' but I pointed out that in a realistic
working copy, you may get thousands of lines of output.

Daniel suggested to leave the behavior of 'svn status' as-is, unless
'svn status' is run with both -v and -q.

I'm not really sure how this morphed into a discussion about 'svn
info', tab-delimited values, RFC822, XML, and feature branches, but if
I may back up for a moment and talk about changelists again...

I think this is a bug. 'svn status' does not print unmodified items in
the "default" changelist, and that is as it should be. For
consistency, unmodified items in an explicit changelist shouldn't be
printed either.

But how do you know what changelists you have and which files you've
added to them? I don't like the idea of doing that with 'svn status -
vq' or 'svn info' because:

Properties have their own 'list' command, 'svn proplist'.

Shelves have their own 'list' command, 'svn x-shelf-list'.

In my mind, listing changelists (and which files are "in" them) should
be a documented part of the changelist feature, not something achieved
by a non-obvious invocation of 'svn status'.

'svn changelist --list' is not congruent to the way properties and
shelves are listed; i.e., those have a separate top-level command like
'svn proplist'. So, perhaps 'svn changelists' (plural)? That lines up
with 'svn x-shelves', but I don't like that it differs only by one
letter from 'svn changelist'. Perhaps 'svn changelist' or 'svn
changelists' with no additional parameters should print the
changelists and their files instead of the current behavior which
outputs E205001?

Nathan

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Julian Foad <ju...@apache.org>.
Branko Čibej wrote:
>>> We might want to allow using multiple options, like "--show-item=revision
>>> --show-item=kind" in addition to comma-separated values.
>> I don't think this is high priority.
> 
> *And* it would require a major rewrite of our argument parser, whereas
> adding a comma-separated list format would not, 

Major rewrite?  Why?

> and would also be compatible with -r and -c.

Whereas other existing options including '--search' and '--changelist' 
already work the first way (accumulating values from multiple options).

- Julian

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Thu, 07 Nov 2019 20:01 +00:00:
> On 07.11.2019 20:25, Daniel Shahaf wrote:
> >> We might want to allow using multiple options, like "--show-item=revision
> >> --show-item=kind" in addition to comma-separated values.
> > I don't think this is high priority.
> 
> *And* it would require a major rewrite of our argument parser, whereas
> adding a comma-separated list format would not, and would also be
> compatible with -r and -c.

"Major"?  We could just change opt_state.show_item from «const char *»
to an array, and parse multiple --show-item arguments the same way we
parse «svn log -r 42 -r 43».  I think it'd be a trivial patch, as no other
subcommand uses --show-item, but its cost/benefit ratio would be
pretty high.

Cheers,

Daniel

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Branko Čibej <br...@apache.org>.
On 07.11.2019 20:25, Daniel Shahaf wrote:
> Julian Foad wrote on Thu, Nov 07, 2019 at 17:53:43 +0000:
> We would then have «svn info --rfc822
> --show-item=depth» that prints "Depth: empty" (with the RFC822 header)

Nitpick: it's not really RFC822 because there are spaces in the
"headers" ...

[...]

>> We might want to allow using multiple options, like "--show-item=revision
>> --show-item=kind" in addition to comma-separated values.
> I don't think this is high priority.

*And* it would require a major rewrite of our argument parser, whereas
adding a comma-separated list format would not, and would also be
compatible with -r and -c.

-- Brane


Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
[...]> I don't see how any of these visions — leaving aside whether 
they're good ideas
> or not — is a blocker to the patch I posted.  You're saying we could add other
> features, and I'm sure we could, but that's not the right question to ask.  The
> question to ask is whether the patch adds value, and whether it adds something
> that we won't want to support until 2.x.  I don't see any concern of the sort
> in all your points.  The patch as it stands is forward-compatible with all your
> ideas.
[...]

Thank you for listening to my thoughts.  I am not going to block the patch.

- Julian

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Thu, Nov 07, 2019 at 17:53:43 +0000:
> We have said the outputs (except XML) simply are not intended to be parsed.

We've said that?  I think what we said is that the non-XML outputs _may_ be
parsed, but with care, since they may change in minor releases as functionality
gets added.  (Example: when we added tree conflicts, we added a 7th column to
the output of 'svn st'.)  That's why we say «%d lines» at the end of every
«svn log | grep '^r[0-9]* '» line.

> Tab-separated output seems a rather arbitrary addition to the flock, though
> it's an OK choice in isolation for this particular use.

The rationale was to offer a line-based format for use in shell scripts and
pipelines.  Shell scripts are basically the reason we added --show-item in the
first place, too.

We could use NUL separators instead of tabs.

> Better consistency with other subcommands would be achieved by using
> space-separated output with field widths chosen per field, like we chose
> them for "status" and "list".

That's not possible in this case: just consider «--show-item=foo,url,relative-url,bar».

> Or item-per-line format like the original "info" output.

Firstly, I'm not this would be worth implementing; the default 'info' output
serves well enough for that.

Secondly, that would be an unusual/unnatural interface for scripts.  Line-based
formats usually operate on the premise that all lines are parsed the same way.
(Example: the output of «netstat».)  «svn info», with or without the RFC822
headers at the start of each line, is the exception to the rule.  The
corresponding parsing idioms (for example, Python's 'fileinput' module, and
«for line in open('foo')») operate one line at a time, not N lines at a time.

> Why should selecting items not be orthogonal to the output format?

There's a distinction between the default and XML outputs on the one hand,
and TSV output on the other hand: In the latter it's harder to select just
the particular fields one cares about.

Without the ability to select items to be output, a consumer of the
tab-separated format that cares just about a particular field would have to
either hardcode magic numbers (as in «awk 'print $1, $13'») or deal with
a header line (meaning the parsing would need to be stateful — exactly the
problem we started with — and things like «xargs -n1 svn info
--show-item=foo,bar --» would print multiple header lines).  On the other hand,
in the default and XML formats selecting just the fields one cares about is easy.

So, selecting output fields is more important for TSV than for RFC822 or XML.

> The XML output should also support --show-item.  It's arbitrarily
> inconsistent that it presently doesn't.

See above: consumers of the XML output can easily ignore the parts they don't
care about, even without --show-item support.  On the other hand, I could see
a case for having «svn info --rfc822 --show-item=foo,bar», which would generate
the default output format but print only some of the lines, for interactive use.

More generally, I see your point that selecting fields and selecting output
format should be orthogonal.  That does make sense from an abstract (pure
mathematics) point of view, but more pragmatically, I don't think it's as high
priority to support «--show-item=… --xml» as to to support multiple arguments
to «--show-item=…» in TSV mode.  In fact, with info-cmd.c as it stands, supporting
«--show-item=… --xml» would not _reduce_ complexity but _increase_ it, since
several different receiver functions would need to become aware of --show-item.

> These days, JSON would be a reasonable choice of output format as an
> alternative to XML.  Consider if we offered it, how would we select it?
> Perhaps a --json flag?  Then why not a --tsv flag for TSV format,

Yes, we could do this.  We could add --json and --tsv, as well as --rfc822 that
would select the default output mode explicitly.  We could generalize that to
--output-mode=(rfc822|json|xml|tsv).  We could make --show-item work in
conjunction with any of json/xml/tsv/rfc822.  For compatibility reasons, «svn
info» would default to --rfc822, but --show-item would imply --tsv unless one
of --json/--xml/--rfc822 was passed.  We would then have «svn info --rfc822
--show-item=depth» that prints "Depth: empty" (with the RFC822 header) and no
other lines, and we'd be able to do «svn info --json --show-item=foo» to save a
couple of microseconds to the process on the other end of the pipe.

I don't see how any of these visions — leaving aside whether they're good ideas
or not — is a blocker to the patch I posted.  You're saying we could add other
features, and I'm sure we could, but that's not the right question to ask.  The
question to ask is whether the patch adds value, and whether it adds something
that we won't want to support until 2.x.  I don't see any concern of the sort
in all your points.  The patch as it stands is forward-compatible with all your
ideas.

> and expect that too to be made available to other subcommands?

Which other commands might use TSV?

> We might want to allow using multiple options, like "--show-item=revision
> --show-item=kind" in addition to comma-separated values.

I don't think this is high priority.

Daniel

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> Branko Čibej wrote on Thu, Nov 07, 2019 at 17:28:35 +0100:
>> Not with a new option. Instead --show-item should accept a
>> comma-separated list of keywords.
> 
> You mean, like this?

It's tempting to say +1, commit it, and say we've made an improvement. 
I have been missing the ability to specify multiple arguments to 
--show-item, and it scratches that itch.

But I do have some reservations and further thoughts.

I feel sad about how inconsistent our output formats are.  Subcommands 
such as "status" and "list" are formatted mainly using fixed-width 
columns, "info" and "log" mainly using separate lines, then there are 
the XML variants, and now a tab-delimited output.  And, in all of these 
except XML, no consistent escaping of values for machine readability. 
We have said the outputs (except XML) simply are not intended to be 
parsed.  That is fine as far as it goes, but leaves something lacking.

Tab-separated output seems a rather arbitrary addition to the flock, 
though it's an OK choice in isolation for this particular use.

Better consistency with other subcommands would be achieved by using 
space-separated output with field widths chosen per field, like we chose 
them for "status" and "list".  Or item-per-line format like the original 
"info" output.  Why should selecting items not be orthogonal to the 
output format?

The XML output should also support --show-item.  It's arbitrarily 
inconsistent that it presently doesn't.  Element names used in XML 
should match those in show-item; compatibility is harder to achieve for 
that, because we already made them different.

These days, JSON would be a reasonable choice of output format as an 
alternative to XML.  Consider if we offered it, how would we select it? 
Perhaps a --json flag?  Then why not a --tsv flag for TSV format, and 
expect that too to be made available to other subcommands?

We might want to allow using multiple options, like 
"--show-item=revision --show-item=kind" in addition to comma-separated 
values.

- Julian

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Thu, 07 Nov 2019 17:15 +00:00:
> +++ subversion/tests/cmdline/info_tests.py	(working copy)
> @@ -644,6 +644,10 @@ def info_item_simple(sbox):
> +  svntest.actions.run_and_verify_svn(
> +    ['1\tdir'], [],
> +    'info', '--show-item=revision,kind', '--no-newline',
> +    sbox.ospath(''))

I've gone ahead and committed this in r1869578, but if there are any
concerns or suggestions, they're still welcome.

Cheers,

Daniel

Re: svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Thu, Nov 07, 2019 at 17:28:35 +0100:
> Not with a new option. Instead --show-item should accept a
> comma-separated list of keywords.

You mean, like this?

[[[
% subversion/svn/svn info wc/foo --show-item=kind,schedule,relative-url 
dir     add     ^/foo
% 
]]]

That's not a mock-up:

[[[
Index: subversion/svn/info-cmd.c
===================================================================
--- subversion/svn/info-cmd.c	(revision 1869499)
+++ subversion/svn/info-cmd.c	(working copy)
@@ -411,8 +411,8 @@ typedef struct print_info_baton_t
    * The following fields are used by print_info_item().
    */
 
-  /* Which item to print. */
-  info_item_t print_what;
+  /* Which items to print. */
+  apr_array_header_t *print_whats;
 
   /* Do we expect to show info for multiple targets? */
   svn_boolean_t multiple_targets;
@@ -436,7 +436,7 @@ typedef struct print_info_baton_t
  * temporary allocation.
  */
 static svn_error_t *
-find_print_what(const char *keyword,
+find_print_what(const char *keywords_from_argv,
                 print_info_baton_t *receiver_baton,
                 apr_pool_t *scratch_pool)
 {
@@ -445,7 +445,9 @@ static svn_error_t *
   svn_cl__simcheck_t *kwbuf = apr_palloc(
       scratch_pool, info_item_map_len * sizeof(svn_cl__simcheck_t));
   apr_size_t i;
-
+  apr_array_header_t *whats;
+  
+  /* Initialize KEYWORDS and KWBUF. */
   for (i = 0; i < info_item_map_len; ++i)
     {
       keywords[i] = &kwbuf[i];
@@ -454,9 +456,14 @@ static svn_error_t *
       kwbuf[i].data = &info_item_map[i];
     }
 
-  switch (svn_cl__similarity_check(keyword, keywords,
-                                   info_item_map_len, scratch_pool))
+  whats = svn_cstring_split(keywords_from_argv, ",", TRUE, scratch_pool);
+  for (i = 0; i < whats->nelts; i++)
     {
+      const char *keyword = APR_ARRAY_IDX(whats, i, const char *);
+      switch (svn_cl__similarity_check(keyword, keywords,
+                                       info_item_map_len, scratch_pool))
+      /* TODO: indent this block */
+    {
       const info_item_map_t *kw0;
       const info_item_map_t *kw1;
       const info_item_map_t *kw2;
@@ -463,8 +470,9 @@ static svn_error_t *
 
     case 0:                     /* Exact match. */
       kw0 = keywords[0]->data;
-      receiver_baton->print_what = kw0->print_what;
-      return SVN_NO_ERROR;
+      APR_ARRAY_PUSH(receiver_baton->print_whats, info_item_t) =
+        kw0->print_what;
+      break;
 
     case 1:
       /* The best alternative isn't good enough */
@@ -503,6 +511,13 @@ static svn_error_t *
             " did you mean '%s', '%s' or '%s'?"),
           keyword, kw0->keyword.data, kw1->keyword.data, kw2->keyword.data);
     }
+    }
+
+  if (whats->nelts == 0)
+    return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                            _("--show-item requires a non-empty argument"));
+
+  return SVN_NO_ERROR;
 }
 
 /* A callback of type svn_client_info_receiver2_t.
@@ -1125,12 +1140,18 @@ print_info_item(void *baton,
          receiver_baton->path_prefix, target, pool));
   const char *const target_path =
     (receiver_baton->multiple_targets ? actual_target_path : NULL);
+  int i;
 
   if (receiver_baton->start_new_line)
     SVN_ERR(svn_cmdline_fputs("\n", stdout, pool));
 
-  switch (receiver_baton->print_what)
+  for (i = 0; i < receiver_baton->print_whats->nelts; i++)
     {
+      if (i > 0) 
+        SVN_ERR(svn_cmdline_fputs("\t", stdout, pool));
+      switch (APR_ARRAY_IDX(receiver_baton->print_whats, i, info_item_t))
+      /* TODO: indent this block */
+    {
     case info_item_kind:
       SVN_ERR(print_info_item_string(svn_node_kind_to_word(info->kind),
                                      target_path, pool));
@@ -1235,6 +1256,7 @@ print_info_item(void *baton,
     default:
       SVN_ERR_MALFUNCTION();
     }
+    }
 
   receiver_baton->start_new_line = TRUE;
   return SVN_NO_ERROR;
@@ -1300,6 +1322,7 @@ svn_cl__info(apr_getopt_t *os,
   else if (opt_state->show_item)
     {
       receiver = print_info_item;
+      receiver_baton.print_whats = apr_array_make(pool, 1, sizeof(info_item_t));
 
       if (opt_state->incremental)
         return svn_error_create(
Index: subversion/svn/svn.c
===================================================================
--- subversion/svn/svn.c	(revision 1869499)
+++ subversion/svn/svn.c	(working copy)
@@ -431,6 +431,7 @@ const apr_getopt_option_t svn_cl__options[] =
                           "                             "
                           "current revision (recommended when tagging)")},
   {"show-item", opt_show_item, 1,
+    /* TODO: update docstring */
                        N_("print only the item identified by ARG:\n"
                           "                             "
                           "   'kind'       node kind of TARGET\n"
Index: subversion/tests/cmdline/info_tests.py
===================================================================
--- subversion/tests/cmdline/info_tests.py	(revision 1869499)
+++ subversion/tests/cmdline/info_tests.py	(working copy)
@@ -644,6 +644,10 @@ def info_item_simple(sbox):
     ['1'], [],
     'info', '--show-item=revision', '--no-newline',
     sbox.ospath(''))
+  svntest.actions.run_and_verify_svn(
+    ['1\tdir'], [],
+    'info', '--show-item=revision,kind', '--no-newline',
+    sbox.ospath(''))
 
 
 def info_item_simple_multiple(sbox):
]]]

In addition to those TODO comments there's also a function docstring that needs
updating.

> And we could add a catch-all keyword, e.g., 'all', as shorthand for 'print
> all attributes as tab-separated values'.

I suppose we could, but I don't see what a script might use --show-item=all for.

Cheers,

Daniel

svn status tabbed output [was: Re: svn status should not show unmodified files in changelists]

Posted by Branko Čibej <br...@apache.org>.
On 07.11.2019 12:13, Daniel Shahaf wrote:

> P.S. Come to think of it, a tab-separated output mode for «svn info» might be
> nice; that is, instead of this:
> .
>     % svn info README
>     Path: README
>     Name: README
>     Working Copy Root Path: /home/daniel/src/svn/t1
>     URL: https://svn.apache.org/repos/asf/subversion/trunk/README
>     Relative URL: ^/subversion/trunk/README
>     Repository Root: https://svn.apache.org/repos/asf
>     Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68
>     Revision: 1869138
>     Node Kind: file
>     Schedule: normal
>     Last Changed Author: luke1410
>     Last Changed Rev: 1746277
>     Last Changed Date: 2016-05-31 14:07:51 +0000 (Tue, 31 May 2016)
>     Text Last Updated: 2019-09-23 18:43:22 +0000 (Mon, 23 Sep 2019)
>     Checksum: 0b8d04c92f856c0d66248cdccf031fb6c0f6e05e
>     Changelist: foo
> .
> we'd have this:
> .
>     % svn info --tsv README
>     README	README	/home/daniel/src/svn/t1	…
>
> This would be one line per file, so it would be easier to parse in a shell
> script.

Not with a new option. Instead --show-item should accept a
comma-separated list of keywords. That's a backwards-compatible change
that I recall being mentioned/requested a time or two (but it's also an
API change). And we could add a catch-all keyword, e.g., 'all', as
shorthand for 'print all attributes as tab-separated values'.

-- Brane


Re: svn status should not show unmodified files in changelists

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, Nov 06, 2019 at 10:36:10 +0000:
> Demo:
> [[[
> $ touch a b c d; svn add a b c d; svn ci -m ""; svn up
> ...
> $ echo hello > b
> $ echo hello > d
> $ svn cl foo c d
> A [foo] c
> A [foo] d
> $ svn status -v
>                  1        1 julianfoad   .
>                  1        1 julianfoad   a
> M                1        1 julianfoad   b
> --- Changelist 'foo':
>                  1        1 julianfoad   c
> M                1        1 julianfoad   d
> $ svn status
> M       b
> --- Changelist 'foo':
>         c
> M       d
> ]]]
> 
> File 'c' shows up in the plain 'svn status' even though it is unmodified.
> 
> I propose we should change this so unmodified files are not listed. It would
> look like this:
> 
> [[[
> $ svn status
> M       b
> --- Changelist 'foo':
> M       d
> ]]]
> 
> A changelist heading is printed only when a file in that changelist is
> listed, so there will be no heading if no files in the changelist are
> modified. It would look like this:
> 
> [[[
> $ svn revert b d
> Reverted 'b'
> Reverted 'd'
> $ svn status -v
>                  1        1 julianfoad   .
>                  1        1 julianfoad   a
>                  1        1 julianfoad   b
> --- Changelist 'foo':
>                  1        1 julianfoad   c
>                  1        1 julianfoad   d
> $ svn status
> ]]]
> 
> Thoughts?

Yes, several.

1. Are you proposing a change to the CLI or to the API?  I assume it's a
CLI-only change (so compatibility is a non-trivial issue).

2. As Nathan said, this can make it difficult to answer "Which unmodified files
are in a changelist?".  (Yes, it's still possible to answer this by parsing
the output of «svn info», but that's far from elegant, particularly since it's
not line-based.)

The root issue here is that "an unmodified file is in a changelist" is an
uncommittable-metadata-only change.  Additions to changelists are explicit user
actions, so I think it's fair to assume they're important and should be shown
by default (and compatibility suggests this too).  However, they aren't
committable, so how about if we made -q hide such files, like it hides
unversioned files?  That is:

        -v passed?      -q passed?      prints unmodified, changelist'd files?
        ==========      ==========      ======================================
                                        1.14    proposed 1.15
                                        ----    -------------
        no              no              yes     yes
        no              yes             yes     no
        yes             no              yes     yes
        yes             yes             yes     no(?)

I kept «svn st» unmodified for compatibility, and by extension «svn st -v» is
unmodified too.  (Presumably «svn st -v» should be a superset of the other
modes.)  I'm not sure about 'svn st -vq', but going by 'svn log -vq', I assume
it should hide unmodified, changelist'd files in the fashion you suggested.
(But then, there's «svn st -quv» to think of…)

Makes sense?  Would this be consistent with how we handle other status values
(unversioned, conflicted, etc)?

Cheers,

Daniel

P.S. Come to think of it, a tab-separated output mode for «svn info» might be
nice; that is, instead of this:
.
    % svn info README
    Path: README
    Name: README
    Working Copy Root Path: /home/daniel/src/svn/t1
    URL: https://svn.apache.org/repos/asf/subversion/trunk/README
    Relative URL: ^/subversion/trunk/README
    Repository Root: https://svn.apache.org/repos/asf
    Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68
    Revision: 1869138
    Node Kind: file
    Schedule: normal
    Last Changed Author: luke1410
    Last Changed Rev: 1746277
    Last Changed Date: 2016-05-31 14:07:51 +0000 (Tue, 31 May 2016)
    Text Last Updated: 2019-09-23 18:43:22 +0000 (Mon, 23 Sep 2019)
    Checksum: 0b8d04c92f856c0d66248cdccf031fb6c0f6e05e
    Changelist: foo
.
we'd have this:
.
    % svn info --tsv README
    README	README	/home/daniel/src/svn/t1	…

This would be one line per file, so it would be easier to parse in a shell
script.

Re: svn status should not show unmodified files in changelists

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> Julian Foad wrote on Wed, Nov 06, 2019 at 16:07:23 +0000:
>> Perhaps one aspect of being in a mature and stable phase of a project's life
>> means that if we are going to change how an existing feature works, we
>> should not change it one little bit at a time like we did in early
>> development phases.  Instead we should plan and release a set of related
>> changes all together.  Instead of just changing "svn status" to not show
>> unmodified files, we should plan a set of improvements to changelists, and
>> that would be one of the changes.
> 
> How do you figure this?  What would be the benefit?
> 
> If you say that we should plan ahead and keep the big picture in mind, I'd
> agree; but it sounds like you're saying that small, incremental changes and
> not letting the perfect be the enemy of the good were appropriate approaches
> for 0.x, while now is the time for making larger refactorings all at once…

Using the changelists UI as the example, I'm imagining if we change the 
behaviour of "svn status" in the way I suggested, in one release, and in 
the next release we found that made it less convenient to discover all 
assigned changelists (though still possible through "status -v") so we 
add an explicit way like "svn cl --list"... then in the next release we 
add a way to select "the unnamed changelist" explicitly... and so on... 
I feel users would get fed up with the apparently "random" bits of churn 
in the feature in each release, and not consider this a stable product 
to work with.

In particular, I might personally have some ideas of a set of related 
changes that would lead to a better overall UX for changelists, but then 
I might get as far as committing one or two increments and then never 
get around to the rest, leaving the feature looking just pointlessly 
modified with the real end benefits never realized.

If we are talking about non-LTS releases, we might well consider that is 
in the nature and purpose of those releases to proceed incrementally and 
release whatever fixes and improvements we get around to committing. 
But that in itself will not result in a well planned set of changes 
being ready for the next LTS release.

On the other hand, if we somehow arrange to make a (LTS, let's say) 
release which says "The changelists feature has been overhauled. It was 
deficient in a number of ways, so we have made a number of changes to 
it, which lets it work better in <these> ways while being almost exactly 
backwards compatible. Exceptions are <these>." ... That seems to me like 
a better delivery.

That's to better explain what I was thinking.  To take this thought to 
the extreme, we would develop all changes on a branch until we think the 
branch contains a complete new version of changelists.  I don't know if 
we would want to go that far.  At least, though, it seems to me that it 
would be wise to plan a group of changes that make sense all together, 
and discuss whether we are happy with what the feature would look like 
at the end, before we start making incremental changes.  Then at least 
if we make one or two changes and get stalled, we will have a collective 
memory of where it was leading so someone else can continue in the same 
direction instead of going off in a different direction.

- Julian

Re: svn status should not show unmodified files in changelists

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, Nov 06, 2019 at 16:07:23 +0000:
> Perhaps one aspect of being in a mature and stable phase of a project's life
> means that if we are going to change how an existing feature works, we
> should not change it one little bit at a time like we did in early
> development phases.  Instead we should plan and release a set of related
> changes all together.  Instead of just changing "svn status" to not show
> unmodified files, we should plan a set of improvements to changelists, and
> that would be one of the changes.

How do you figure this?  What would be the benefit?

If you say that we should plan ahead and keep the big picture in mind, I'd
agree; but it sounds like you're saying that small, incremental changes and
not letting the perfect be the enemy of the good were appropriate approaches
for 0.x, while now is the time for making larger refactorings all at once…

Did I misunderstand?

Cheers,

Daniel

Re: svn status should not show unmodified files in changelists

Posted by Julian Foad <ju...@apache.org>.
Nathan Hartman wrote:
> "svn status" (without -v) is defined as showing only locally modified
> items. So I wouldn't object to that, provided there's a way to list
> changelist(s) in effect and what files are in them.

Well, "svn status -v" would still show them, but I agree with your point 
in general.

I wrote about a package of general improvements to changelist support, 
including "svn changelist --list", here:

 
https://cwiki.apache.org/confluence/display/SVN/Shelve-Checkpoint+Side+Issues#Shelve-CheckpointSideIssues-ChangelistUIImprovements

I should add two more items to that page:
   - svn status shouldn't show unmodified files
   - need "svn info --show-item=changelist PATH..."

Perhaps one aspect of being in a mature and stable phase of a project's 
life means that if we are going to change how an existing feature works, 
we should not change it one little bit at a time like we did in early 
development phases.  Instead we should plan and release a set of related 
changes all together.  Instead of just changing "svn status" to not show 
unmodified files, we should plan a set of improvements to changelists, 
and that would be one of the changes.

- Julian

Re: svn status should not show unmodified files in changelists

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Nov 6, 2019 at 5:36 AM Julian Foad <ju...@apache.org> wrote:

> svn status doesn't generally show unmodified files, except with '-v'.
> However, in changelists, it shows unmodified files, even without '-v'


> I am finding this to be a inconsistent, and a nuisance because I am
> currently working on a task in which I have a lot of files potentially
> modified, in two or more groups which I want to commit separately,
> scattered across paths. The use case seems right for changelists, but
> "svn status" shows a screenful of non-changed files.


"svn status" (without -v) is defined as showing only locally modified
items. So I wouldn't object to that, provided there's a way to list
changelist(s) in effect and what files are in them.

E.g.
"svn changelist --list"
or
"svn changelist --show"
or
"svn status --changelists"
or something.

Otherwise you'd have to remember what you've done--and the whole point
of version control is not to have to remember! :-)

Nathan