Support`arc browse --revision <commit>` and make `arc browse` with no arguments mean `... --revision HEAD`
Open, Needs TriagePublic

Description

In order to launch a browser on a revision today, the user must use the form:

arc browse Dnnn

A common operation is to want to open the browser to whatever revision is associated with the branch you're working on, without first having to run a separate command (such as arc feature and look for your branch, or git log).

For our users, this is noticeable delta because we used to have:

name_of_internal_tool open

Which would do exactly this (open the appropriate Review Board review request).

A possible way to support this nicely with arc is to allow use of browse with no further arguments:

arc browse

Today this merely gives an error message:

Usage Exception: Specify one or more paths or objects to browse. Use the command "arc browse ." if you want to browse this directory.

I understand that arc browse is intended to be multi-purpose. My naive argument for this would be that if you're trying to browse a commit or a directory, you by definition know what you are wanting to browse - whether it's passing . or HEAD or similar.

On the other hand, if you're trying to browse a revision there is a fundamental gap between what you clearly know as a user and what you need to instruct arc to do what you want. As a result, it feels somewhat reasonable to me that the default behavior without arguments is to assume the user wants to browse a revision, and detect the revision of the current branch.

An alternative might be:

arc browse D.    # notice the dot ;)

Or some similar syntax that's easy to type and remember, yet also avoids implying to the user that arc browse is only useful for revisions.

scode created this task.Apr 28 2016, 5:58 PM
scode updated the task description. (Show Details)
scode added a project: Restricted Project.Apr 28 2016, 6:00 PM
scode moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Apr 28 2016, 6:00 PM

Some earlier discussion in T5781, although I didn't really have a clear way forward at the time and still don't.

There's also arc diff --browse but that does arc diff first, then browses the thing it just created/updated, so it's only useful in a much narrower set of conditions. We could possibly arc diff --browse --just-browse-do-not-diff but that's kind of silly.

You can arc branch, then arc browse Dxxx to do the lookup relatively quickly, but not as quickly as a hypothetical arc browse --the-revision-for HEAD.

I do think that if we were to assign a behavior to arc browse with no arguments, arc browse --the-revision-for HEAD is the most reasonable behavior.

It's probably reasonable to just add --revision (maybe with short flag -D?), hard-code it into arc browse, and make arc browse with no arguments mean arc browse --revision HEAD. No more-general use cases have really arisen since T5781 that I can recall.

scode added a comment.Apr 28 2016, 6:45 PM

My gut feeling is that our users will feel that arc browse --the-revision-for HEAD is too verbose for something that to them is a common and "should be simple" operation. However, if combined with the default behavior as you suggest, such that just arc browse defaults appropriately it would avoid that being a problem while keeping the meaning of the --the-revision-for HEAD option clear.

(As an aside not sure if you meant it literally, but I'd vote for arc browse --revision-for HEAD - removing the the.)

I'm not entirely clear on the distinction between --revision HEAD and --the-revision-for HEAD. If you just switched because --the-revision-for is too verbose, then +1 :)

Oh, the actual implementation would be --revision (with a short option), I was just using --the-revision-for for clarity (or extra confusion?) in discussing the meaning/behavior of the flag.

You'd actually type:

$ arc browse --revision <branch|commit> # Browse revision for head of <branch|commit>.
$ arc browse -D <branch|commit> # Short version, "-D" like "Dnnn"? But maybe this is silly and "-r" is easier to remember.
$ arc browse # Means `arc browse --revision HEAD` in Git.
scode added a comment.Apr 28 2016, 6:58 PM

Ah, sorry about that!

Sounds good.

On -D vs. -r:

Well, -r is more obvious if you already know of --revision. If you don't know anything, you're probably not going to guess -D nor -r. If you see -D in a command you might be more likely to be confused by the argument value not being a number, since you're used to seeing the Dnnn form. So perhaps -r is preferable? No strong opinion here.

epriestley renamed this task from easy browsing of revision of current branch to Support`arc browse --revision <commit>` and make `arc browse` with no arguments mean `... --revision HEAD`.Apr 29 2016, 1:53 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.May 21 2016, 4:05 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jun 1 2016, 10:39 PM
avivey added a subscriber: avivey.Jun 1 2016, 10:55 PM

I don't remember where, but at some point I explicitly asked that arc browse with no arguments will not browse anything, to allow [my] workflow of find-some-files | xargs arc browse. At the time, it was suggested that it would default to arc browse . (I think it only knew about paths at the time).

Do you type that explicitly, or would something like find-some-files | xargs arc browse --paths ("force any arguments to be interpreted as paths") be good enough?

Maybe --force could even just do that, without needing a new argument, since it already forces argument interpretation as paths? So the behavior would be:

$ arc browse --force
No paths provided.
$ arc browse
# Magic, tries to guess what you might mean.

I currently have b aliased to arc browse, so I type something like gfg TrainStation | x b.

I guess I can add --paths to b while I'm at it.

Maybe --force should be renamed to --paths then, because it's clearer?

Yeah, --force is a little weird as a name given what it actually does.

I'm also still not truly sold on arc browse doing magic. As an alternative, we could improve the default behavior to suggest both arc browse --revision HEAD (and explain what that does, if it would do something) and arc browse . (as we currently offer). I generally think there are a lot of tools available for users who want to optimize keystrokes (like shell aliases and arc alias) and it's questionable to privilege them over new users.

Stuff after T5055, etc., should also make it easier to distribute workflow and aliases and ship with arc b to really get those keystrokes down to a minimum.

Also, for future-me: arc browse XYZ (invalid object name) has an escaping problem.

eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:17 PM
epriestley moved this task from Backlog to vNext on the Arcanist board.
epriestley moved this task from Backlog to The Queue on the Prioritized board.Nov 22 2016, 10:38 PM

After D16933, the behavior of arc browse <branch> on the experimental branch is:

  • If the HEAD of branch is present in the remote (specifically: in Diffusion, in Phabricator), but not part of an open revision, browse the commit.
  • If the HEAD of branch is part of an open revision, but not present in the remote, browse the revision.
  • If the HEAD of branch is part of both, prompt the user to browse the revision or commit.
  • You can use --types to limit the query to a subset of resolvers (for example, --types revision will never interpret the argument as a commit).
  • If there is no argument, HEAD is tested according to the rules described above, provided the user is in a working copy.

So arc browse will usually browse a revision, and arc browse HEAD will usually browse a revision too, unless HEAD has already been pushed. Then you get a commit or a prompt, depending You can use arc alias with --types to create a browse command with different resolution rules.

There are likely still issues with resolution in general (see T11355) and various rough edges that will get cleaned up as I work through broader arc issues.

(If you name your branches ambiguous stuff like P123 you'll be prompted to choose between an object, commit, or branch interpretation and can use --types to narrow the query -- or just name your branches something unambiguous.)