D5299: phabricator: fallback reading arcanist config files

philpep added a comment. Thanks for reviewing @Kwan ! Actually I think parsing/merging arcconfig files will be quite hard to maintain because they are not fully documented and subject to changes. For my needs, just having hg being able to get phabricator.url and

D5299: phabricator: fallback reading arcanist config files

durin42 requested changes to this revision. durin42 added a comment. This revision now requires changes to proceed. I see at least one outstanding comment that points out a typo. I'm generally in favor of this though, so please let us know when it's time to take another look. REPOSITORY

D5299: phabricator: fallback reading arcanist config files

Kwan added inline comments. INLINE COMMENTS > philpep wrote in phabricator.py:175 > I'd be nice to cache the result of `readarcconfig` but I don't known how to > implement this, any suggestion ? Looks like annotating it with `@util.cachefunc` is enough? > Kwan wrote in phabricator.py:217-222

D5299: phabricator: fallback reading arcanist config files

Kwan added inline comments. INLINE COMMENTS > Kwan wrote in phabricator.py:201 > I think you could do > > url = util.url(conduit_uri) > url.path = None > url = b'%s' % url > > Also I think you need to check for `phabricator.uri` first, then fall back to > `conduit_uri`, see change to

D5299: phabricator: fallback reading arcanist config files

Kwan added inline comments. INLINE COMMENTS > philpep wrote in phabricator.py:201 > HINT: This doesn't work for current mercurial config because "arc > install-certificates" add a trailing "/" to conduit_uri and our .arcconfig > doesn't have this trailing slash. > Any idea how to handle this

D5299: phabricator: fallback reading arcanist config files

philpep marked 3 inline comments as done. philpep added inline comments. INLINE COMMENTS > mharbison72 wrote in phabricator.py:187 > s/.encoding/.environ/ ? Woops... Thanks for catching this! > mharbison72 wrote in phabricator.py:200 > Should this be using vfs to open, instead of raw open?

D5299: phabricator: fallback reading arcanist config files

philpep updated this revision to Diff 14253. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5299?vs=14175=14253 REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git

D5299: phabricator: fallback reading arcanist config files

mharbison72 added inline comments. INLINE COMMENTS > phabricator.py:187 > +paths = [ > +vfsmod.vfs(encoding.encoding['ProgramData']).join( > + 'Phabricator', 'Arcanist', 'config'), s/.encoding/.environ/ ? > phabricator.py:200 > +if

D5299: phabricator: fallback reading arcanist config files

philpep marked 2 inline comments as done. philpep added a comment. Thanks for the review and sorry for taking so long time to update the patch... I made changes to use encoding.environ and vfs like you suggested. Please let me known what I can do to introduce this (or at least part of)

D5299: phabricator: fallback reading arcanist config files

philpep updated this revision to Diff 14175. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5299?vs=14174=14175 REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git

D5299: phabricator: fallback reading arcanist config files

philpep updated this revision to Diff 14174. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5299?vs=12590=14174 REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git

D5299: phabricator: fallback reading arcanist config files

mharbison72 added inline comments. INLINE COMMENTS > phabricator.py:179 > +""" > +if os.name == 'nt': > +paths = [ pycompat.iswindows > phabricator.py:181 > +paths = [ > +os.path.join(os.environ['ProgramData'], > + 'Phabricator', I

D5299: phabricator: fallback reading arcanist config files

philpep added inline comments. INLINE COMMENTS > phabricator.py:175 > +def readarcconfig(repo): > +"""Return url, token, callsign read from arcanist config files > + I'd be nice to cache the result of `readarcconfig` but I don't known how to implement this, any suggestion ? REPOSITORY

D5299: phabricator: fallback reading arcanist config files

philpep added inline comments. INLINE COMMENTS > phabricator.py:201 > +if conduit_uri is not None: > +token = config.get('hosts', {}).get(conduit_uri, {}).get('token') > +url = conduit_uri.rstrip('/api/') HINT: This doesn't work for current mercurial config because "arc

D5299: phabricator: fallback reading arcanist config files

philpep created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY This allow the phabricator extension to read arc config files to auto-configure url, token and callsign. We use it as a fallback when phabricator.url or