Better handling of `.arcconfig` files.
ClosedPublic
Actions

Authored by joshuaspence on Jun 23 2014, 4:05 AM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

rARC750a94e89fad: Better handling of `.arcconfig` files.

Summary

Throw a useful error message when an .arcconfig file is not valid JSON.

Depends on D9697.

Test Plan

Modified an .arcconfig file to be invalid JSON.

> arc lint
Usage Exception: Your '~/.arcrc' file is not a valid JSON file.

Parse error on line 18 at column 4: Expected: 'STRING' - It appears you have an extra trailing comma

Diff Detail

Repository

rARC Arcanist

Branch

arcconfig-error

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1319
Build 1319: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence updated this revision to Diff 23229.Jun 23 2014, 4:05 AM

joshuaspence retitled this revision from to Better handling of `.arcconfig` files..

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJun 23 2014, 4:05 AM

Herald added subscribers: Korvin, epriestley. · View Herald Transcript

Harbormaster completed remote builds in B1319: Diff 23229.Jun 23 2014, 4:06 AM

Maybe worth considering PhutilProxyException or introducing some UsageProxyException. Not sure how nasty the PhutilProxyException output is.

This revision is now accepted and ready to land.Jun 23 2014, 2:12 PM

See D9679#13

Use a PhutilProxyException

Please confirm that you are okay with the use of PhutilProxyException.

Harbormaster completed remote builds in B1341: Diff 23265.Jun 23 2014, 7:09 PM

epriestley accepted this revision.Jun 23 2014, 11:22 PM

epriestley edited edge metadata.

This revision is now accepted and ready to land.Jun 23 2014, 11:22 PM

Closed by commit rARC750a94e89fad (authored by @joshuaspence).

Since updating to this revision, I get this error:

$ arc linters
Exception
Failed to parse '.../site/.git/arc/config' as JSON.
Parse error on line 1 at column 0: Expected one of: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['
(Run with --trace for a full exception trace.)

And that's because:

$ cat .git/arc/config
{}

Should an empty .git/arc/config really be an error? I think arc made that itself at some point.

In D9681#22, @turadg wrote:
Since updating to this revision, I get this error:
$ arc linters
Exception
Failed to parse '.../site/.git/arc/config' as JSON.
Parse error on line 1 at column 0: Expected one of: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['
(Run with --trace for a full exception trace.)
And that's because:
$ cat .git/arc/config
{}
Should an empty .git/arc/config really be an error? I think arc made that itself at some point.

No, this doesn't seem right. Let me take a look.

In D9681#22, @turadg wrote:
Since updating to this revision, I get this error:
$ arc linters
Exception
Failed to parse '.../site/.git/arc/config' as JSON.
Parse error on line 1 at column 0: Expected one of: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['
(Run with --trace for a full exception trace.)
And that's because:
$ cat .git/arc/config
{}
Should an empty .git/arc/config really be an error? I think arc made that itself at some point.