D220: rpms: add chg

2017-08-03 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Seems fine, queued, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D220 To: Mathiasdm, #hg-reviewers, yuja Cc: yuja, mercurial-devel

D299: py3: introduce a wrapper for __builtins__.{raw_,}input()

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > encoding.py:585 > +def __init__(self, buffer, **kwargs): > +kwargs[r'encoding'] = encoding > +super(strio, self).__init__(buffer, **kwargs) Fixed as `s/encoding/_sysstr(encoding)/`. REPOSITORY rHG Mercurial REVISION DETAIL

D360: log: add a "graphwidth" template variable

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > cmdutil.py:2557 > displayer.flush(ctx) > -edges = edgefn(type, char, lines, state, rev, parents) > -for type, char, lines, coldata in edges: > +for type, char, width, coldata in itertools.chain([firstedge], > edges):

D351: demandimport: disable by default if chg is being used

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added a comment. > Setting `HGDEMANDIMPORT` for both the forked and non-forked chg client processes seems less cleaner. That seems less bad since the hg script doesn't have to know the chg stuff. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D351 To:

D299: py3: introduce a wrapper for __builtins__.{raw_,}input()

2017-08-15 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > util.py:180 > +sys.stdin, sys.stdout = encoding.strio(fin), encoding.strio(fout) > +return pycompat.bytestr(input(*args, **kwargs)) > +else:

D300: python3: whitelist four more passing tests

2017-08-15 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. This appears to be identical to https://phab.mercurial-scm.org/D298. Dropped. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D300 To: durin42,

D350: demandimport: test whether to enable or not in hg script

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added a comment. > `sys.modules['hgdemandimport']` being set? Maybe I should remove it from commit message. So there's no point to duplicate the HGDEMANDIMPORT check? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D350 To: quark, #hg-reviewers Cc:

D98: revset: support reading aliases from a .hgrevsets file

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added a comment. > I couldn't tell from Yuya's comment if "... not add such revset functions" means I meant "don't load unsafe extensions." revsets and filesets have to be secure against malicious input because they are accessible through the Web UI. It's up to users to make a

D351: demandimport: disable by default if chg is being used

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added a comment. Perhaps the chg executable can simply set `HGDEMANDIMPORT=disable` if it isn't specified by user. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D351 To: quark, #hg-reviewers Cc: yuja, mercurial-devel

D350: demandimport: test whether to enable or not in hg script

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added a comment. Seems fine, but what is the "side effects caused by importing demandimport" ? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D350 To: quark, #hg-reviewers Cc: yuja, mercurial-devel ___

D352: chgserver: special handle __version__ in mtimehash (issue5653)

2017-08-15 Thread yuja (Yuya Nishihara)
yuja added a comment. I doubt the issue5653 had this scenario. Waiting for more detailed bug report. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D352 To: quark, #hg-reviewers Cc: yuja, mercurial-devel ___

D360: log: add a "graphwidth" template variable

2017-08-14 Thread yuja (Yuya Nishihara)
yuja added a comment. > I had avoided this change because it might break extensions with their own edgefns. Do you think that's a concern? It's probably okay to change the function interface. I don't think the current `edgefn` is any useful for extensions. And I heard no problem after

D243: obsmarker: rename precnode into prednode

2017-08-14 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > lothiraldan wrote in cmdutil.py:1916 > I'm not sure to see which template keyword you are talking about. `fm.write('prednode', ...)` REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D243 To: lothiraldan, #hg-reviewers,

D296: extensions: if on py3 and propname is a bytestr, convert to sysstr

2017-08-12 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > yuja wrote in extensions.py:388 > `pycompat.sysstr()` ? The "if" shouldn't be necessary. That's what `sysstr()` is for. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D296 To: durin42, #hg-reviewers Cc: yuja,

D299: pycompat: introduce a wrapper for __builtins__.{raw_,}input()

2017-08-12 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > hgk.py:100 > try: > -line = raw_input().split(' ') > +line = pycompat.bytesinput(sys.stdin, sys.stdout).split(' ') > node1 = line[0] Perhaps hgk should use ui.fin and ui.fout. > durin42

D229: phabricator: update diff property even if we choose not to create a new diff

2017-08-12 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > phabricator.py:280 > diff = creatediff(ctx) > writediffproperties(ctx, diff) > transactions.append({'type': 'update', 'value': diff[r'phid']}) `writediffproperties()` is called twice. Perhaps one of them should be deleted.

D242: context: rename troubled into isunstable

2017-08-12 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > context.py:246 > +self._repo.ui.deprecwarn(msg, '4.4') > +return self.isunstable() > + Please send a follow-up. This is too far from the tip to amend. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D242

D243: obsmarker: rename precnode into prednode

2017-08-12 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > cmdutil.py:1916 > fm.write('index', '%i ', index) > -fm.write('precnode', '%s ', hex(marker.precnode())) > +fm.write('precnode', '%s ', hex(marker.prednode())) > succs = marker.succnodes() The template keyword should be updated

D318: wireproto: remove support for local results in @batchable (API)

2017-08-13 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > peer.py:99 > encargsorres, encresref = next(batchable) > if not encresref: > return encargsorres # a local result in this case Perhaps this could be deleted too. REPOSITORY rHG Mercurial REVISION DETAIL

D252: revset: rename bumped into phasedivergent

2017-08-13 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > revset.py:472 > > -Only non-public and non-obsolete changesets can be `bumped`. > +Only non-public and non-obsolete changesets can be `phasedivergent`. > """ Maybe these revset functions should be marked as `(EXPERIMENTAL)`?

D360: log: add a "graphwidth" template variable

2017-08-13 Thread yuja (Yuya Nishihara)
yuja added a comment. > The edge > drawing function needs to know the number of lines in the template output, so > we need to also determine how wide that drawing would be before we call the > edgefn or evaluate the template. Actually `asciiedges()` doesn't need the `lines` to

D421: hg: move part of top-level logic to dispatch.run

2017-08-17 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > hg:30 > try: > -if sys.version_info[0] < 3 or sys.version_info >= (3, 6): > -import hgdemandimport; hgdemandimport.enable() > +import

D352: chgserver: special handle __version__ in mtimehash (issue5653)

2017-08-17 Thread yuja (Yuya Nishihara)
yuja added a comment. I'll drop this per your comment on the issue5653. Thanks for investigating it further. https://bz.mercurial-scm.org/show_bug.cgi?id=5653#c4 REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D352 To: quark, #hg-reviewers Cc: yuja,

D98: revset: support reading aliases from a .hgrevsets file

2017-08-10 Thread yuja (Yuya Nishihara)
yuja added a comment. > Security-wise, the "shelling out revset" seems hard to solve cleanly. Yes, and IMHO the only reasonable solution is to not add such revset functions. revsets and filesets can be executed through the web UI. REPOSITORY rHG Mercurial REVISION DETAIL

Re: D97: revset: pass repo when passing ui

2017-07-18 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > cmdutil.py:2307 > +matcher = revset.match(repo.ui, expr, repo=repo, > + order=revset.followorder) > revs = matcher(repo, revs) Perhaps these two shouldn't pass `ui` nor `repo`. They evaluate internal

Re: D117: run-tests: warn if --color=always and no pygments installed

2017-07-18 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > run-tests.py:427 > + 'pygments is not installed\n') > > global useipv6 Nit: missing space after "because". REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D117 EMAIL PREFERENCES

Re: D98: revset: support reading aliases from a .hgrevsets file

2017-07-20 Thread yuja (Yuya Nishihara)
yuja added a comment. > What else should we allow in here? Are there security concerns we should think through? Probably safe, but could be used for DoS attack: - `filesetalias` (not implemented) - `revsetalias` - `committemplate` - `templatealias` - `templates` Unsafe

Re: D98: revset: support reading aliases from a .hgrevsets file

2017-07-19 Thread yuja (Yuya Nishihara)
yuja added a comment. Can we have a single source file for project's revset/fileset aliases and maybe templates? I don't think it's good idea to create `.hgxxx` file per config section. [revsetalias] filealiasall = all() [filesetalias] ... Perhaps they could be

D260: chg: define _GNU_SOURCE to allow CentOS 5 compilation

2017-08-08 Thread yuja (Yuya Nishihara)
yuja accepted this revision as: yuja. yuja added a comment. Looks good. The manpage says > The O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW flags are not specified in POSIX.1-2001, but are specified in > POSIX.1-2008. Since glibc 2.12, one can obtain their definitions by defining either

D260: chg: define _GNU_SOURCE to allow CentOS 5 compilation

2017-08-08 Thread yuja (Yuya Nishihara)
yuja added a comment. I've queued these for stable so 4.3 rpm can be built for CentOS 5. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D260 To: Mathiasdm, #hg-reviewers, yuja Cc: yuja, quark, mercurial-devel ___

D299: pycompat: introduce a wrapper for __builtins__.{raw_,}input()

2017-08-09 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > pycompat.py:84 > +setattr(sys, stream, noclosetextio(s)) > +return bytestr(input(*args, **kwargs)) > +finally: Needs to specify encoding because user input may contain non-ascii characters. Perhaps it should

D296: extensions: if on py3 and propname is a bytestr, decode as ascii

2017-08-09 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > extensions.py:388 > +if pycompat.ispy3 and isinstance(propname, bytes): > +propname = propname.decode('ascii') > assert callable(wrapper) `pycompat.sysstr()` ? > extensions.py:401 > raise AttributeError( >

D271: obsolete: use bytestr() instead of str() so the node is bytes on py3

2017-08-09 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > obsolete.py:587 > > -marker = (str(prec), tuple(succs), int(flag), metadata, date, > parents) > +prec = bytes(pycompat.bytestr(prec)) > +marker = (prec, tuple(succs), int(flag), metadata, date, parents) Why can't this be

D223: color: remove warnings if term is not formatted (==dumb or !ui.formatted())

2017-08-04 Thread yuja (Yuya Nishihara)
yuja accepted this revision as: yuja. yuja added a comment. Makes sense. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D223 To: spectral, #hg-reviewers, yuja Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list

D441: revset: optimize "draft() & ::x" pattern

2017-08-19 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > quark wrote in revsetlang.py:492 > Yes! That's much shorter (I had to copy the trees from debugshell) and more > robust (no fragile `len(x) >= 3` handling). > > For performance concerns, I think it's fine. Practically, revset parsing is > hardly a

D441: revset: optimize "draft() & ::x" pattern

2017-08-18 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > revsetlang.py:492 > + 'follow'))) > +if matched: > +return w, ('func', ('symbol', '_phaseandancestors'), FWIW, I wrote placeholder matcher once, which could be used as follows. m = _matchtree('_() & ::_',

D351: demandimport: disable if chg is being used

2017-08-19 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued this, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D351 To: quark, #hg-reviewers, phillco, yuja Cc: phillco, sid0, yuja, mercurial-devel

D447: templatekw: choose {latesttag} by len(changes), not date (issue5659)

2017-08-19 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > templatekw.py:222 > +# Don't call expensive key function if not necessary > +pdate, pdist, ptag = ptags[0] > except

D447: templatekw: choose {latesttag} by len(changes), not date (issue5659)

2017-08-19 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > martinvonz wrote in templatekw.py:222 > > This should be enabled only if ptags[0][2] != ptags[1][2]. > > Good point. Done. > > > The latesttagdistance is documented to return the longest path to the > > latest tag. > > I'm not changing

D447: templatekw: choose {latesttag} by len(changes), not date (issue5659)

2017-08-19 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. This one looks good to me, thanks. I leave it to Sean since he said he's queued the previous series. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D447 To:

D451: revset: remove order information from tree

2017-08-19 Thread yuja (Yuya Nishihara)
yuja added a comment. Clever. I haven't looked this carefully, but the general direction seems fine. > @yuja Let me know if this can simplify `matchtree`, `buildtree` implementation. Actually `matchtree` can ignore extra elements in a node tuple, so the existence of `order` flag

D451: revset: remove order information from tree

2017-08-22 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D451#7257, @yuja wrote: > So, `anyorder` for `not x` would be my mistake because `x and not y` could be > theoretically flipped. FWIW, this should be okay since `not x` follows the order of the input subset. The order

D451: revset: remove order information from tree

2017-08-22 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D451#7281, @quark wrote: > I think it's more correct if all core revsets support `defineorder` explicitly. The current code depends on `revset.makematcher` using ascending fullreposet as default to make `defineorder` implicitly

D455: test-revset: make it work with chg

2017-08-23 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued this one, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D455 To: quark, #hg-reviewers, yuja Cc: yuja, mercurial-devel

D451: revset: remove order information from tree

2017-08-23 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D451#7499, @quark wrote: > hgsubversion: > > - svnrev() uses `x for x in myset if x in subset` therefore enforces "defineorder" and wrong. > - fromsvn() is also wrong similarly. Yes, they are wrong (and I've pointed

D351: demandimport: disable if chg is being used

2017-08-17 Thread yuja (Yuya Nishihara)
yuja added a comment. Can you update the comment in chgserver.py in new series? It says "CHGINTERNALMARK is temporarily set by chg client to detect if chg will start another chg." REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D351 To: quark, #hg-reviewers,

D360: log: add a "graphwidth" template variable

2017-08-17 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Slightly adjusted the commit message for new version, and queued, thanks. INLINE COMMENTS > hooper wrote in cmdutil.py:2557 > There can be many of them. E.g. in the hg repo it looks like a

D351: demandimport: disable if chg is being used

2017-08-18 Thread yuja (Yuya Nishihara)
yuja added a comment. > Instead of running `hg serve --cmdserver chgunix ...`, chg client executes `chgserve ...`. And the hg script could notice that by checking `sys.argv[0]` and does some special handling At this point, I don't think it's simpler than using CHGINTERNALMARK or

D663: dirstate: perform transactions with _map using single call, where possible

2017-09-13 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > dirstate.py:554 > +entry = self._map.get(d) > +if entry is not None and entry != 'r': > raise error.Abort(

D696: registrar: add a enum 'cmdtype' for the type of the command

2017-09-15 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. To make it less controversial, I would move these constants to registrar.command class and rename them to lowercaseconstants. The registrar provides semi-public API, which should

D663: dirstate: perform transactions with _map using single call, where possible

2017-09-15 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D663 To: mbolin, #hg-reviewers, phillco, yuja Cc: yuja, phillco, mercurial-devel

D623: copytrace: move fast heuristic copytracing algorithm to core

2017-09-14 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. > I don't think we can just swap c1 and c2 because what we're calculating > is the copy from c1 to c2, For the record, this statement appears to be wrong, sorry.

D724: templater: extract shortest() logic from template function

2017-09-16 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. I slightly prefer moving shortest() to scmutil as it seems not a core function of revlog, but that's just my preference. REPOSITORY rHG Mercurial REVISION DETAIL

D657: revset: move weight information to predicate

2017-09-20 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. Queued, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D657 To: quark, #hg-reviewers, phillco, krbullock, singhsrb, yuja Cc: krbullock, yuja, phillco, mercurial-devel

D724: templater: extract shortest() logic from template function

2017-09-17 Thread yuja (Yuya Nishihara)
yuja added a comment. > \What I originally thought was `shortest` may have alternative implementation if the changelog is not backed by revlog (in the far far future). Okay, got it. Even if it's backed by revlog, shortest can be implemented as a one-pass lookup of prefix tree.

D680: scmutil: handle conflicting files and dirs in origbackuppath

2017-09-13 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > scmutil.py:570 > +# Remove any files that conflict with the backup file's path > +for f in util.finddirs(fullorigpath): > +if

D735: uncommit: add an experimental.uncommitondirtywdir config

2017-09-22 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > uncommit.py:158 > if len(wctx.parents()) > 1: > raise error.Abort(_("cannot uncommit while merging")) > old = repo['.'] Perhaps we should test this case, dirtyuncommit & merge. REPOSITORY rHG Mercurial REVISION

D656: revset: remove "small" argument from "_optimize"

2017-09-19 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. (copied from the email thread) On Fri, 8 Sep 2017 17:06:12 +, quark (Jun Wu) wrote: >> yuja wrote in revsetlang.py:421 > > Any reason to choose 1 here and

D657: revset: move weight information to predicate

2017-09-19 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. (copied from the email thread, though I'm okay with the latest patch.) On Fri, 8 Sep 2017 18:24:44 +, phillco (Phil Cohen) wrote: > phillco added a comment. > >

D654: blackbox: unindent a try block

2017-09-19 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. This look like doing more than "unindent a try block". INLINE COMMENTS > blackbox.py:153 > +changed = '' > +if ui._bbrepo: > +ctx =

D680: scmutil: handle conflicting files and dirs in origbackuppath

2017-09-19 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > mbthomas wrote in scmutil.py:570 > I've changed it to use `util.normpath`, which I think converts back to `/` as > separator on windows (`windows.normpath` calls `windows.pconvert`), and is > probably a good idea anyway. Is this sufficient?

D652: blackbox: simplify ui states

2017-09-19 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Looks good. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D652 To: quark, #hg-reviewers, durham, yuja Cc: yuja, mercurial-devel

D654: blackbox: unindent a try block

2017-09-20 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued the series, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D654 To: quark, #hg-reviewers, durham, yuja Cc: yuja, mercurial-devel

D960: bundle2: immediate exit for ctrl+c (issue5692)

2017-10-06 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > quark wrote in bundle2.py:380 > Actually `KeyboardInterrupt` is tested here. So there is something else going > on. Maybe we should add `error.SignalInterrupt` to the list? SignalInterrupt is a subclass of KeyboardInterrupt, so there might be

D943: chg: move only first time relevant if condition out of loop

2017-10-06 Thread yuja (Yuya Nishihara)
yuja added a comment. What I don't like is this change increases complexity of the loop condition, checking argc at two places, and incrementing i at two places, conditionally. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D943 To: singhsrb, #hg-reviewers,

D965: templatefilters: be sure we always feed cgi.escape a str

2017-10-06 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. sysstr -> sysbytes can't be round-trip if non-ascii characters are involved. Perhaps we can just vendor cgi.escape() and add b'' to every string. REPOSITORY rHG Mercurial

D964: python3: add and use adapter for func_name attr, now known as __name__

2017-10-06 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. Can't we just use `func.__name__` on both Pythons? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D964 To: durin42, #hg-reviewers, pulkit, indygreg,

D959: patch: invalidate messages after encoding change

2017-10-08 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. So the messages may be rolled back, which seems to be against the original intent, "take messages out of the function so that extensions can add entries." Since extensions have

D924: config: add a missing preparewrite() call

2017-10-04 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D924 To: quark, #hg-reviewers, yuja Cc: yuja, mercurial-devel

D912: test-alias: make it compatible with chg

2017-10-04 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. The test output doesn't look right. Maybe we should disable the test on chg. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D912 To: quark,

D917: eol: make [eol] config section sensitive for chg confighash

2017-10-04 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. It's a core extension. Let's simply add it to chgserver.py. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D917 To: quark, #hg-reviewers, yuja Cc: yuja,

D987: copies: add a config to limit the number of candidates to check in heuristics

2017-10-09 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. - ui.configint() ? - I don't think taking the first 5 candidates would do something sensible. Is it reasonable? - tests? REPOSITORY rHG Mercurial REVISION DETAIL

D986: copies: add docs for config `experimental.copytrace.sourcecommitlimit`

2017-10-09 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. queued, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D986 To: pulkit, #hg-reviewers, yuja Cc: yuja, mercurial-devel

D1081: tests: add check in test-check-commit.t to verify releasenotes directives

2017-10-14 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. It makes a file named "notes" in the repository because of cd $TESTDIR/... Perhaps we should send the output to > /dev/null. REPOSITORY rHG Mercurial REVISION DETAIL

D1079: hgweb: rewrite most obviously-native-strings to be native strings

2017-10-14 Thread yuja (Yuya Nishihara)
yuja added a comment. Please don't count on me that I can catch unicode issues around Python URL/HTTP libraries. I hate Python 3 because of the unicode mess, so I don't write it for any projects other than Mercurial. Maybe we'll need a clear boundary between Mercurial and Python

D1080: hgweb: fix logging to use native strings as appropriate

2017-10-14 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > server.py:339 > raise error.Abort(_("cannot start server at '%s:%d': %s") > - % (address, port, inst.args[1])) > +

D1081: tests: add check in test-check-commit.t to verify releasenotes directives

2017-10-15 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. Looked good, but it appears that releasenotes requires fuzzywuzzy. + abort: No module named fuzzywuzzy.fuzz! (on gcc112) Another problem is it doubles the execution

D1028: mpatch: reformat function prototypes with clang-format

2017-10-13 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. Seemed fine, but test-check-code.t says no. + mercurial/mpatch.c:268: + > ssize_t start, ssize_t end) + don't use spaces to indent +

D958: i18n: clean msgcache when encoding changes

2017-10-13 Thread yuja (Yuya Nishihara)
yuja added a comment. I've sent an alternative idea as https://phab.mercurial-scm.org/D1053. Can you take a look? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D958 To: quark, #hg-reviewers, lothiraldan Cc: yuja, ryanmce, lothiraldan, mercurial-devel

D1053: i18n: cache translated messages per encoding

2017-10-13 Thread yuja (Yuya Nishihara)
yuja created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY This is a simpler workaround alternative to https://phab.mercurial-scm.org/D958, "i18n: clean msgcache when encoding changes." The cache won't be bloated unless you

D1096: releasenotes: make the import of fuzzywuzzy optional

2017-10-16 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > releasenotes.py:222 > +try: > +import fuzzywuzzy.fuzz as fuzz > +except ImportError: Needs to evaluate the fuzz module to get around the

D1097: releasenotes: don't abort is there is a bad formatted entry for releasenotes

2017-10-16 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > releasenotes.py:327 > if not paragraphs: > -raise error.Abort(_('could not find content for release note > ' > -

D1095: amend: add a flag `-n/--note` to store note with amend

2017-10-16 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Seems fine, though I don't know how the note will be displayed. FWIW, note cannot be a long sentence due to the storage limit. $ hg amend --note "$(yes | head -300 | tr -d '\n')"

D943: chg: move only first time relevant if condition out of loop

2017-10-05 Thread yuja (Yuya Nishihara)
yuja added subscribers: quark, yuja. yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. I prefer the original code since it looks simpler and there wouldn't be measurable performance win. @quark which do you like? INLINE

D886: ui: convert to/from Optional[bytes] to Optional[str] in password manager

2017-10-05 Thread yuja (Yuya Nishihara)
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D886 To: durin42, #hg-reviewers, yuja Cc: yuja, mercurial-devel

D1095: amend: add a flag `-n/--note` to store note with amend

2017-10-16 Thread yuja (Yuya Nishihara)
yuja added a comment. > We don't have a UI to display obsmarker in core, so for now extensions has to do this. I know. It would be nice if reviewers can know how the planned UI will be to determine whether only --note option should be in the next release or not. REPOSITORY rHG

D1112: hgweb: fix decodevaluefromheaders to always return a bytes value

2017-10-16 Thread yuja (Yuya Nishihara)
yuja added a comment. Perhaps bytesurl()/strurl() should be renamed to e.g. asciibytes()/asciistr() since we've started using them everywhere non-ascii character must be rejected. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1112 To: durin42,

D987: copies: add a config to limit the number of candidates to check in heuristics

2017-10-16 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D987#18592, @stash wrote: > > copy tracing is disabled > > means that it's disabled only for this particular file and not for all files? Yes, this particular file which has an excessive number of candidates. REPOSITORY

D1121: releasenotes: show a warning if fuzzywuzzy is not present

2017-10-17 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > releasenotes.py:104 > +ui.warn(_("module 'fuzzywuzzy' not found, merging of similar" > + " releasenotes is disbaled")) > + Fixed missing \n and typo. REPOSITORY rHG Mercurial REVISION DETAIL

D1099: dagutil: use a listcomp instead of a map()

2017-10-17 Thread yuja (Yuya Nishihara)
yuja added a comment. I have no preference, but pycompat.maplist() could be used. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1099 To: durin42, #hg-reviewers, ryanmce Cc: yuja, ryanmce, mercurial-devel ___

D1137: templater: don't blow up when trying to build an abort message

2017-10-17 Thread yuja (Yuya Nishihara)
yuja added a comment. pycompat.sysbytes() can be used here. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1137 To: durin42, #hg-reviewers, ryanmce Cc: yuja, ryanmce, mercurial-devel ___ Mercurial-devel mailing list

D1124: test-hgweb-annotate-whitespace: make test compatible with chg

2017-10-17 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D1124#18881, @singhsrb wrote: > @durin42: That's a valid concern and I am planning to look at the pattern handling code at some point. Maybe we should instead make `hg serve --daemon` work under commandserver. Argument

D1119: releasenotes: fix documentation of similaritycheck()

2017-10-17 Thread yuja (Yuya Nishihara)
yuja added a comment. I guess the original intent was s/merged to/included in/. The return value is named "merge", which is set to False if similarity is high. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1119 To: pulkit, #hg-reviewers, durin42 Cc: yuja,

D1136: templatefilters: defend against evil unicode strs in json filter

2017-10-17 Thread yuja (Yuya Nishihara)
yuja requested changes to this revision. yuja added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ryanmce wrote in templatefilters.py:241-242 > I fear that this is is an non-actionable error. How might I see it as a user? > If I see it, what do I do? > >

D1024: hgweb: do not import uuid immediately to avoid its side effect

2017-10-13 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > test-dispatch.t:64 > + > +#if no-windows > + Replaced with `#if rmcwd` REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1024 To: quark, #hg-reviewers, av6 Cc: yuja, mercurial-devel

D987: copies: add a config to limit the number of candidates to check in heuristics

2017-10-13 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D987#17308, @pulkit wrote: > In https://phab.mercurial-scm.org/D987#17141, @yuja wrote: > > > - do not try to find copy from the first n candidates, and show the message saying copy tracing is disabled due to too many

D1065: python3: move from using func_name to __name__

2017-10-14 Thread yuja (Yuya Nishihara)
yuja added a comment. This is queued, but we'll need to wrap __name__ with sysbytes() for py3. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1065 To: durin42, #hg-reviewers, yuja Cc: mercurial-devel ___

D887: httppeer: use native strings for headers

2017-10-14 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > durin42 wrote in httppeer.py:240 > Turns out urlencode already copes with bytes input even on Python 3. A small > victory. Nice. So we have to concatenate bytes url and bytes qs, then covert the result to unicode. REPOSITORY rHG Mercurial

D1053: i18n: cache translated messages per encoding

2017-10-13 Thread yuja (Yuya Nishihara)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGd00ec62d156f: i18n: cache translated messages per encoding (authored by yuja, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D1053?vs=2670=2694

D656: revset: remove "small" argument from "_optimize"

2017-09-08 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > revsetlang.py:421 > elif op == 'rangeall': > -return smallbonus, x > +return 1, x > elif op in ('rangepre', 'rangepost', 'parentpost'): Any reason to choose 1 here and 0.5 for the others? REPOSITORY rHG Mercurial

  1   2   3   4   5   6   7   8   9   >