git-arc: -t tag support for create to set Phabricator project tags

[dteske]

Summary

Add -t tag[,...] so a review can be tagged at creation instead of
needing the web UI. Spaces in tag names are written as underscores;
a leading # is optional.

[dteske]

In creating this review, I tested the new feature:

git arc create -t src_committers -r markj,adrian HEAD

It worked. The tag was pre-associated with the review without having to go to the web interface.

[markj]

tools/tools/git/git-arc.sh

61

All of the subcommands take a -h now, not just create.

487

Why is this needed? We've already checked this at the bottom of the script. Ditto in gitarc__list().

927

Why do all the individual subcommands handle -h now too? Isn't this one check sufficient?

[dteske]

tools/tools/git/git-arc.sh

61

All of the subcommands take a -h now, not just create.

Correct. Because here's what git arc does right now:

dteske@FreeBSD git $ git arc create -h
usage: git arc 
dteske@FreeBSD git $ git arc update -h
usage: git arc

Sub-optimal to say the least.

You then have to guess and hope that "git arc -h" is how you can see the synopsis without loading up the man-page.

After this patch:

dteske@FreeBSD git $ git arc create -h
Usage: git arc [-hvy] <command> <arguments>

Commands:
  create [-dhl] [-r <reviewer1>[,<reviewer2>...]] [-s subscriber[,...]] \
                [-t tag[,...]] [<commit>|<commit range>]
  diff <commit>|<commit range>
  list <commit>|<commit range>
  patch [-bcrs] <diff1> [<diff2> ...]
  stage [-b branch] [<commit>|<commit range>]
  update [-l] [-m message] [<commit>|<commit range>]

See git-arc(1) for details.

(same output for git arc update -h and other sub-commands).

Now it's actually helpful.

1. It actually shows you the synopsis for the sub-command
2. It also points you to the man-page for more information (in case you either didn't know there was a man-page or if you didn't know it's git-arc(1) instead of git_arc(1))

I stopped short of adding a new per-sub-command usage in favor of keeping the diff short and concise; just chose to echo the main usage when sub-commands get -h

[markj]

tools/tools/git/git-arc.sh

61

I'm pointing out that you added -h to the usage string for create, but not the other subcommands, and I don't see a reason for the inconsistency.

[dteske]

tools/tools/git/git-arc.sh

487

Why is this needed? We've already checked this at the bottom of the script. Ditto in gitarc__list().

Good catch. That's on me. I added those checks *before* the check on line 931 to implement "Too few arguments" and then later adjusted line 931 to also implement the same message (if memory serves).

I will remove these.

While I was working through your questioning here, I came to the realization that the usage statement shows a synopsis that is incongruent with reality.

According to the synopsis from usage (reproduced below):

Commands:
  create [-dhl] [-r <reviewer1>[,<reviewer2>...]] [-s subscriber[,...]] \
                [-t tag[,...]] [<commit>|<commit range>]
  diff <commit>|<commit range>
  list <commit>|<commit range>
  patch [-bcrs] <diff1> [<diff2> ...]
  stage [-b branch] [<commit>|<commit range>]
  update [-l] [-m message] [<commit>|<commit range>]

The display of [<commit>|<commit range>] at the end of create, stage, and update sub-commands (suggesting optional argument) flies in the face of two things:

First, the man-page synopsis directly contradicts the usage synopsis (indicating required argument):

git arc create [-dl] [-r reviewer1[,reviewer2 ...]]
        [-s subscriber1[,subscriber2 ...]] [-p parent]
        commit-ref [commit-ref ...]
git arc diff commit-ref [commit-ref ...]
git arc list commit-ref [commit-ref ...]
git arc patch [-bcrs] diff1 [diff2 [...]]
git arc stage [-b branch] commit-ref [commit-ref ...]
git arc update [-l] [-m message] commit-ref [commit-ref ...]

Coupled with this comment from the code:

# All subcommands require at least one parameter.

I will, while removing the unnecessary double-check of $#, fix up the usage synopsis to align with man-page synopsis and code comment.

[adrian]

devin, let's split the 'handle -h and provide better help' change from the tag handling change.

[dteske]

tools/tools/git/git-arc.sh

61

Good catch. I'll adjust the synopsis to advertise -h for all the sub-commands, and make that update in both the command usage and man-page for consistency (while also fixing the inconsistency between command/man-page synopsis mentioned just-previously)

[dteske]

In D59019#1354277, @adrian wrote:

devin, let's split the 'handle -h and provide better help' change from the tag handling change.

Sigh. Sure. (grumble grumble extra work grumble)

I would *prefer* to handle it immediately and ASAP because the below user experience is beyond deplorable:

$ git arc create -h
Usage: git arc
$ git arc update -h
Usage: git arc
$ git arc -h
(actually helpful usage)

[dteske]

tools/tools/git/git-arc.sh

927

Why do all the individual subcommands handle -h now too? Isn't this one check sufficient?

Remove this and then this happens:

dteske@FreeBSD git $ git arc create -h
usage: git arc

Comment explains why. Before sourcing git-sh-setup, we set USAGE to empty:

USAGE=
# shellcheck disable=SC1090
. "$git_sh_setup"

Prior to this check, there is only a shift of the verb, no shift of the "-h" which means that the check of $# is insufficient

[dteske]

Fix synopsis to align with man-page/code. Refactor.

Preceding breakout-out of usage updates.

[dteske]

Advertise -h in all sub-commands.

Again, preceding break-out of usage updates.

[dteske]

Only introduce -t

[dteske]

Same but for man-page