Page MenuHomePhabricator

Minor modernizations to `arc browse`
ClosedPublic

Authored by epriestley on Feb 9 2014, 5:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 17, 6:20 AM
Unknown Object (File)
Fri, Dec 13, 4:42 AM
Unknown Object (File)
Wed, Dec 4, 6:55 AM
Unknown Object (File)
Sat, Nov 30, 12:23 AM
Unknown Object (File)
Thu, Nov 28, 6:51 PM
Unknown Object (File)
Thu, Nov 28, 6:51 PM
Unknown Object (File)
Thu, Nov 28, 6:51 PM
Unknown Object (File)
Thu, Nov 28, 6:32 PM
Subscribers
Tokens
"Haypence" token, awarded by btrahan.

Details

Summary

Do a little cleanup:

  • Remove copyright header (we removed all of these a long time ago, this one just snuck through somehow).
  • Remove @group comment (obsolete with new Diviner).
  • Note support for all VCSes.
  • Add pht() for translation.
  • Hint arc browse ..
  • Fail on no paths sooner.
  • Raise a useful error if we can't figure out which repository we're heading to.
  • Clarify "open" comment.
  • Use Filesystem::binaryExists().
  • Some minor wordsmithing.
Test Plan

arc browse, arc browse ., arc browse README, arc browse README src, ran arc browse in valid working copy with no associated repo.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sophiebits added inline comments.
src/workflow/ArcanistBrowseWorkflow.php
66

Curious: any reason you used double quotes here but single quotes on line 35? (And single quotes with embedded backticks on 103?)

src/workflow/ArcanistBrowseWorkflow.php
66

Not really. Contributing factors:

  • I have a reasonably consistent internal single/double quote rule, which is to use single quotes unless a string is more complicated to express with them (for example, it contains "\n" or variables). A theory here is that single quotes are a cue to the reader that the rest of the string isn't interesting/magical, and particularly that it contains no variables, so they can ignore it when tracing a variable. This is a very fuzzy motivation and there are probably plenty of strings that I'd write differently on different days. It also leads to using single quotes or double quotes inside strings (to demarcate values) inconsistently, as I'll sometimes just use whichever one doesn't need to be escaped. As we're pht()'ing the codebase, there's less need for magical strings, so this is getting slightly more consistent. (We could use just one quote style consistently, but this would make some strings dramatically less readable, no matter which style was chosen.)
  • I used to use backticks around commands a lot but have sort of moved away from it. I think it's a little weird and partly just laziness because they never need to be quoted. I'm not really sure if this practice is useful ("backticks delimit commands"), lazy (backticks don't need to be quoted) or just bizarre, or some mixture of the three. I have a fair number of weird idiosyncrasies that I've mostly tried to move away from. For example, I used to indent all the code in every file under the theory that <?php creates a scope. Technically arguably, but not very pragmatic.
  • I am having a pretty fuzzy brain day today.

I'll make these consistent. My reworded exception is probably not very readable anyway because you end up with dot-quote-dot at the end, which is difficult to scan. A better phrasing of this would probably be:

...use:

  $ arc browse .