D9135: doc: Generate separate commands/topics/extension web pages.
ludovicchabant created this revision. Herald added a reviewer: hg-reviewers. Herald added a subscriber: mercurial-patches. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D9135 AFFECTED FILES .hgignore doc/Makefile doc/gendoc.py doc/templates/cmdheader.txt doc/templates/extheader.txt doc/templates/topicheader.txt CHANGE DETAILS diff --git a/doc/templates/topicheader.txt b/doc/templates/topicheader.txt new file mode 100644 --- /dev/null +++ b/doc/templates/topicheader.txt @@ -0,0 +1,4 @@ +.. _topic-%(topicname)s: + +%(topictitle)s + diff --git a/doc/templates/extheader.txt b/doc/templates/extheader.txt new file mode 100644 --- /dev/null +++ b/doc/templates/extheader.txt @@ -0,0 +1,4 @@ +.. _ext-%(extname)s: + +%(exttitle)s + diff --git a/doc/templates/cmdheader.txt b/doc/templates/cmdheader.txt new file mode 100644 --- /dev/null +++ b/doc/templates/cmdheader.txt @@ -0,0 +1,22 @@ +.. _hg-%(cmdname)s.1: + +%(cmdtitle)s + +%(cmdshortdesc)s + +.. contents:: + :backlinks: top + :class: htmlonly + :depth: 1 + +Synopsis + + +:: + + %(cmdsynopsis)s + +Description +--- +%(cmdlongdesc)s + diff --git a/doc/gendoc.py b/doc/gendoc.py --- a/doc/gendoc.py +++ b/doc/gendoc.py @@ -131,7 +131,8 @@ # print help topics # The config help topic is included in the hgrc.5 man page. -helpprinter(ui, helptable, minirst.section, exclude=[b'config']) +topics = findtopics(helptable, exclude=[b'config']) +helpprinter(ui, topics, minirst.section) ui.write(minirst.section(_(b"Extensions"))) ui.write( @@ -166,7 +167,162 @@ ) -def showtopic(ui, topic): +def showcommandlist(ui): +# ui.verbose = True +cmdnames = allcommandnames(table, debugcmds=False) +for mainname, allnames in cmdnames.items(): +ui.write(mainname) +ui.write(b" ") + + +def showtopiclist(ui): +for topic in helptable: +ui.write(topic[0][0]) +ui.write(b" ") + + +def showextensionlist(ui): +for extensionname in allextensionnames(): +ui.write(extensionname) +ui.write(b" ") + + +def showhelpindex(ui): +ui.write(minirst.section(_(b"Mercurial Distributed SCM"))) + +missingdoc = _(b"(no help text available)") + +cats, h, syns = help._getcategorizedhelpcmds(ui, table, None) +ui.write(minirst.subsection(_(b"Commands"))) + +for cat in help.CATEGORY_ORDER: +catfns = sorted(cats.get(cat, [])) +if not catfns: +continue + +catname = gettext(help.CATEGORY_NAMES[cat]) +ui.write(minirst.subsubsection(catname)) +for c in catfns: +url = b'hg-%s.html' % c +ui.write(b" :`%s <%s>`__: %s" % (c, url, h[c])) +syns[c].remove(c) +if syns[c]: +ui.write(b" (aliases: *%s*)" % (b', '.join(syns[c]))) +ui.write(b"\n") +ui.write(b"\n\n") + +ui.write(b"\n\n") + +ui.write(minirst.subsection(_(b"Additional Help Topics"))) +topiccats, topicsyns = help._getcategorizedhelptopics(ui, helptable) +for cat in help.TOPIC_CATEGORY_ORDER: +topics = topiccats.get(cat, []) +if not topics: +continue + +catname = gettext(help.TOPIC_CATEGORY_NAMES[cat]) +ui.write(minirst.subsubsection(catname)) +for t, desc in topics: +url = b'topic-%s.html' % t +ui.write(b" :`%s <%s>`__: %s" % (t, url, desc)) +topicsyns[t].remove(t) +if topicsyns[t]: +ui.write(b" (aliases: *%s*)" % (b', '.join(topicsyns[t]))) +ui.write(b"\n") +ui.write(b"\n\n") + +ui.write(b"\n\n") + +# Add an alphabetical list of extensions, categorized by group. +sectionkeywords = [ +(b"(ADVANCED)", _(b"(ADVANCED)")), +(b"(EXPERIMENTAL)", _(b"(EXPERIMENTAL)")), +(b"(DEPRECATED)", _(b"(DEPRECATED)"))] +extensionsections = [ +(b"Extensions", []), +(b"Advanced Extensions", []), +(b"Experimental Extensions", []), +(b"Deprecated Extensions", [])] +for extensionname in allextensionnames(): +mod = extensions.load(ui, extensionname, None) +shortdoc, longdoc = _splitdoc(mod) +for i, kwds in enumerate(sectionkeywords): +if any([kwd in shortdoc for kwd in kwds]): +extensionsections[i+1][1].append( +(extensionname, mod, shortdoc)) +break +else: +extensionsections[0][1].append( +(extensionname, mod, shortdoc)) +for sectiontitle, extinfos in extensionsections: +ui.write(minirst.subsection(_(sectiontitle))) +for extinfo in sorted(extinfos, key=lambda ei: ei[0]): +extensionname, mod, shortdoc = extinfo +url = b'ext-%s.html' % extensionname +ui.write(minirst.subsubsection( +b'`%s <%s>`__' % (extensionname, url))) +ui.write(sh
D9134: help: extract logic for listing commands and topics.
ludovicchabant created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D9134
AFFECTED FILES
mercurial/help.py
CHANGE DETAILS
diff --git a/mercurial/help.py b/mercurial/help.py
--- a/mercurial/help.py
+++ b/mercurial/help.py
@@ -638,6 +638,55 @@
return re.sub(br'( *)%s' % re.escape(marker), sub, doc)
+def _getcategorizedhelpcmds(ui, cmdtable, name, select=None):
+# Category -> list of commands
+cats = {}
+# Command -> short description
+h = {}
+# Command -> string showing synonyms
+syns = {}
+for c, e in pycompat.iteritems(cmdtable):
+fs = cmdutil.parsealiases(c)
+f = fs[0]
+syns[f] = fs
+func = e[0]
+if select and not select(f):
+continue
+doc = pycompat.getdoc(func)
+if filtercmd(ui, f, func, name, doc):
+continue
+doc = gettext(doc)
+if not doc:
+doc = _(b"(no help text available)")
+h[f] = doc.splitlines()[0].rstrip()
+
+cat = getattr(func, 'helpcategory', None) or (
+registrar.command.CATEGORY_NONE
+)
+cats.setdefault(cat, []).append(f)
+return cats, h, syns
+
+
+def _getcategorizedhelptopics(ui, topictable):
+# Group commands by category.
+topiccats = {}
+syns = {}
+for topic in topictable:
+names, header, doc = topic[0:3]
+if len(topic) > 3 and topic[3]:
+category = topic[3]
+else:
+category = TOPIC_CATEGORY_NONE
+
+topicname = names[0]
+syns[topicname] = list(names)
+if not filtertopic(ui, topicname):
+topiccats.setdefault(category, []).append(
+(topicname, header)
+)
+return topiccats, syns
+
+
addtopichook(b'config', inserttweakrc)
@@ -760,31 +809,7 @@
return rst
def helplist(select=None, **opts):
-# Category -> list of commands
-cats = {}
-# Command -> short description
-h = {}
-# Command -> string showing synonyms
-syns = {}
-for c, e in pycompat.iteritems(commands.table):
-fs = cmdutil.parsealiases(c)
-f = fs[0]
-syns[f] = b', '.join(fs)
-func = e[0]
-if select and not select(f):
-continue
-doc = pycompat.getdoc(func)
-if filtercmd(ui, f, func, name, doc):
-continue
-doc = gettext(doc)
-if not doc:
-doc = _(b"(no help text available)")
-h[f] = doc.splitlines()[0].rstrip()
-
-cat = getattr(func, 'helpcategory', None) or (
-registrar.command.CATEGORY_NONE
-)
-cats.setdefault(cat, []).append(f)
+cats, h, syns = _getcategorizedhelpcmds(ui, commands.table, name,
select)
rst = []
if not h:
@@ -805,7 +830,7 @@
cmds = sorted(cmds)
for c in cmds:
if ui.verbose:
-rst.append(b" :%s: %s\n" % (syns[c], h[c]))
+rst.append(b" :%s: %s\n" % (b', '.join(syns[c]), h[c]))
else:
rst.append(b' :%s: %s\n' % (c, h[c]))
@@ -844,20 +869,7 @@
rst.extend(exts)
rst.append(_(b"\nadditional help topics:\n"))
-# Group commands by category.
-topiccats = {}
-for topic in helptable:
-names, header, doc = topic[0:3]
-if len(topic) > 3 and topic[3]:
-category = topic[3]
-else:
-category = TOPIC_CATEGORY_NONE
-
-topicname = names[0]
-if not filtertopic(ui, topicname):
-topiccats.setdefault(category, []).append(
-(topicname, header)
-)
+topiccats, topicsyns = _getcategorizedhelptopics(ui, helptable)
# Check that all categories have an order.
missing_order = set(topiccats.keys()) - set(TOPIC_CATEGORY_ORDER)
To: ludovicchabant, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] hgweb: don't try to wrap mod_wsgi loggers
> It might be better to test isinstance(stream, io.RawIOBase). If the stream > is a RawIOBase, write() should return the number of bytes written. I'm not super familiar with Python C/native extension code, but it doesn't look to me like the mod_wsgi.Log type is inheriting from an io base class? https://github.com/GrahamDumpleton/mod_wsgi/blob/develop/src/server/wsgi_logger.c#L538 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] hgweb: don't try to wrap mod_wsgi loggers
# HG changeset patch # User Ludovic Chabant # Date 1601413962 25200 # Tue Sep 29 14:12:42 2020 -0700 # Node ID f8726d166fcaa0d690f6783146172758b2e14deb # Parent 80bf7b1ada15622ea45b8ecc5647404f5acb2905 hgweb: don't try to wrap mod_wsgi loggers diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py --- a/mercurial/utils/procutil.py +++ b/mercurial/utils/procutil.py @@ -108,6 +108,10 @@ # The io.BufferedIOBase.write() contract guarantees that all data is # written. return stream +if str(type(stream).__module__) == 'mod_wsgi': + # WSGI loggers write all data. We couldn't even wrap them anyway since + # they typically don't return the number of bytes written. + return stream # In general, the write() method of streams is free to write only part of # the data. return WriteAllWrapper(stream) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] hgdemandimport: exclude more sqlalchemy modules
# HG changeset patch # User Ludovic Chabant # Date 1601415526 25200 # Tue Sep 29 14:38:46 2020 -0700 # Node ID 0b78eb21c79f02980e59ae30c6049075534b # Parent f8726d166fcaa0d690f6783146172758b2e14deb hgdemandimport: exclude more sqlalchemy modules We could potentially exclude the entire sqlalchemy library. diff --git a/hgdemandimport/__init__.py b/hgdemandimport/__init__.py --- a/hgdemandimport/__init__.py +++ b/hgdemandimport/__init__.py @@ -52,6 +52,7 @@ 'rfc822', 'mimetools', 'sqlalchemy.events', # has import-time side effects (issue5085) +'sqlalchemy.dialects', # similar problems as above # setuptools 8 expects this module to explode early when not on windows 'distutils.msvc9compiler', '__builtin__', ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Exceptions in hgdemandimport
We are running into a weird python import error over on hg.sr.ht... see this callstack: https://paste.sr.ht/~sircmpwn/2f1855857494e1e4ee71b7dbd9ef4fca6b99be89 The fix is to force-import the offending type before we enable hgdemandimport: from sqlalchemy.dialects.postgresql.base import CreateEnumType I saw that the hgdemandimport code contains a few exceptions, including one for sqlalchemy.events, already. Maybe it needs to exclude the entirety of sqlalchemy? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D8272: archive: fix crash when archiving to gzip file with Python 3.8.2+
ludovicchabant added a comment. It does seem brittle to me to override some Python internal method anyway? Can we get rid of this altogether? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8272/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8272 To: ludovicchabant, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D8272: archive: fix crash when archiving to gzip file with Python 3.8.2+
ludovicchabant created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REPOSITORY rHG Mercurial BRANCH default REVISION DETAIL https://phab.mercurial-scm.org/D8272 AFFECTED FILES mercurial/archival.py CHANGE DETAILS diff --git a/mercurial/archival.py b/mercurial/archival.py --- a/mercurial/archival.py +++ b/mercurial/archival.py @@ -146,7 +146,7 @@ self.timestamp = timestamp gzip.GzipFile.__init__(self, *args, **kw) -def _write_gzip_header(self): +def _write_gzip_header(self, *args, **kwargs): self.fileobj.write(b'\037\213') # magic header self.fileobj.write(b'\010') # compression method fname = self.name To: ludovicchabant, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve-ext v2] py3: broad pass for python3 compatibility
> > Should opts[x], opts.get(x), etc really use bytes? They usually get passed as > **opts, which means their keys are str, I think. > Sounds OK to me -- there's probably a whole bunch of places that might need to be excluded. It might be worth exploring doing the opposite approach, i.e. manually adding byte strings until things work. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve-ext v2] py3: broad pass for python3 compatibility
> I'm curious to hear what Ludovic thinks, but I'm doubtful that saves > other contributors more time than it costs for Ludovic, so it's a net > negative. > > I assume you know that Python 2's end of life is Jan 1 2020, so this > is getting urgent. I don't know about others, but my team will need to > have Mercurial and extensions on Python 3 by then. At this point it's not too costly for me to update the patch (although it's a bit annoying), especially since I'm not even running any tests yet at this point: I just need a py3 mercurial that doesn't crash when evolve is enabled, and seems to run ok for pull/push. However, I would indeed prefer if someone from Octobus was to take it away from me and deal with it ASAP. It's blocking several new features for sourcehut, and, indeed, py3 EOL is approaching fast and both Drew (lead dev on sourcehut) and Alpine (the distro on which sourcehut runs) have aggressive timelines to completely remove any trace of py2 before that date. Plus, if someone from Octobus was to take this, it could also be part of a broader "kickstart py3 support" task that would include setting up py3 builds on the CI, and other related tasks. Cheers! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve-ext v2] py3: broad pass for python3 compatibility
> This patch don't apply anymore :-/ It is also very invasive and will be > painful for all other contributors. It might be because I created the patch on top of `stable` instead of some other head? To be precise it was applied on top of the 9.0 release since that was the latest on the stable branch last week. > Do you think we could get to a point where no manual tweaking is needed > ? (could be simply adding dedicated comment to line that need to be > skipped). If we have a script only approach we could use the > format-source extensions for that. I already mentioned the steps to reproduce the patch, but there's one more step now: 1. Run `byteify-strings` with `--dictiter` 2. Remove the added `b` prefix where `exthelper.addattr` is used (there should be only 2 places with it in `obsdiscovery.py`). 3. In `templatekw.py` we wrap some functions and re-feed them to `templatekeyword`. Down the callstack it tries to encode the docstring, but it had already been encoded once, so you'll notice 2 more manual edits to decode the docstring back into its original form. To get to a script-only approach, you can submit the 2 changes from step 3 as a separate changeset, and then somehow modify `byteify-strings` so that step 2 isn't necessary... a simple approach would be to add some support for a line-end comment like the `#noqa` that some linters support... so say you add `#nobytestr` at the end of the 2 appropriate lines in `obsdiscovery.py` so that the script skips them. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: python3, hgloader, and extensions
> I don't care much about the correctness of the hgloader. It's a temporary > hack, and Python 3 support is still experimental. I understand it's experimental but you can expect sourcehut to push somewhat aggressively on it :) > > Is the plan to run byteify-strings.py on the mercurial codebase then? > > It's the plan as far as I know. Ok, I just re-sent the patch for the evolve extension then. Thanks! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: python3, hgloader, and extensions
Thanks for the reply! > AFAIK, that's by-design thing. What the hgloader does is a bunch > of weird hacks only valid for our codebase, which we'll eventually > get rid of. Then either the design is wrong, or the code is wrong. First, like I said, depending on *how* you import an extension, it will or will not run the hgloader. That's extremely confusing, and has lost me a few days of investigations and head scratching. The behaviour should be the same whether you use a pip-installed, PYTHONPATH-located, or specific location import. Second, the hgloader specifically looks for (among other things) `hgext3d.*` and `hgext.*`. If we didn't want to run it for 3rd party extensions, it surely is a bug that it checks for `hgext3rd`. Plus, when you import an extension directly by path, it hardcodes its module name to `hgext.` so technically the hgloader _would_ process it (it doesn't right now but it's really by pure luck I think). So if the hgloader is _not_ supposed to run on non-core extensions, we should remove the check for `hgext3rd` at least, and, as much as possible, prevent it from running even when you import an extension that's in your PYTHONPATH. At that point, I think the easiest way would be to change the hgloader to check for the location of the source file and see if it falls inside the core mercurial install path or somewhere else. What do you think? > You can instead use contrib/byteify-strings.py to apply some of the > changes the hgloader would do. Yeah I orginally did that. It makes your code ugly, and it's only even remotely acceptable as an "upgrade" tool. When you actually write a *new* extension (or edit an existing one to add a new command or something), it's totally not natural to write byte strings everywhere. Is the plan to run byteify-strings.py on the mercurial codebase then? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
python3, hgloader, and extensions
For those who missed my latest adventures on IRC, I recently discovered that the hgloader (which patches strings and other py3 things at compile time) doesn't kick in if you load an extension from a specific file-system location. This means that: 1) If you `pip install hg-evolve` and add `[extensions] evolve=` to your config, it (mostly) works. 2) If you `hg clone evolve` and add `[extensions] evolve=/path/to/clone` to your config, mercurial complains that someone is trying to add unicode strings to the command table. You might have noticed I just sent a patch that fixes evolve's `setup.py` file so that we can actually do option 1 until option 2 is fixed. Of course this isn't only for evolve, it's for _any_ 3rd party extension... any random single-file hg extension you might have will likely be imported via option 2, and right now, under py3, you're forced to bytify all your strings and make your code even uglier. I don't like my code being uglier than it should be. Now, I have half of a fix for option 2, which makes sure the hgloader is applied to packages loaded by path, and not just packages loaded "normally". It looks like this: https://paste.sr.ht/%7Eludovicchabant/99176dadaddd53bf9d24ea09682b8fd403e55fdb Of course, it doesn't totally work, otherwise I would have sent it for review: 1) evolve still crashes mercurial with some string encoding issue in the template keywords... I have no idea what's the difference between that and the code path that loads it from the PYTHONPATH. Please help :) 2) It obviously uses Python 3.5+ APIs only so it needs to be improved to work with whatever versions of Python we want to support. I don't know what the core devs want to do here. I'd be grateful to get some help and pointers on both those issues, since I don't really know what I'm doing here (I just learned about meta_paths and loaders like a couple days ago :) ). Cheers! Ludo ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH evolve-ext] py3: make setup.py py3 compatible
# HG changeset patch
# User Ludovic Chabant
# Date 1561756040 0
# Fri Jun 28 21:07:20 2019 +
# Branch stable
# Node ID 90daf413dfc7a7e4762e6445f05c52b123c6188f
# Parent 756db65030c64b22836fe236d1db3b95477e3ef7
py3: make setup.py py3 compatible
diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -7,7 +7,8 @@
def get_metadata():
meta = {}
fullpath = join(dirname(__file__), META_PATH)
-execfile(fullpath, meta)
+with open(fullpath, 'r') as fp:
+exec(fp.read(), meta)
return meta
def get_version():
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve-ext] py3: broad pass for python3 compatibility
> We definitely want this patch, but we equally want a way to test (not > just locally) that it keeps working. Maybe we can better coordinate the > efforts in #hg-evolve. Will do, thanks! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve-ext] py3: broad pass for python3 compatibility
Should I re-create the patch on the latest evolve stable now? Or will someone else do it? I'm asking because py3 support is blocking a few things for sourcehut. Thanks! On Sat, May 25, 2019, at 00:05, Ludovic Chabant wrote: > > Does this make any existing tests for evolve pass on py3? > > The evolve extension wasn't even loading in py3 before this patch -- > mercurial core would reject it for trying to add unicode strings to the > command table. > > > Taking other patches first would mean this patch would be hard to > > apply and it would miss any newly added code, so the question is: how > > hard would it be to create this patch again? > > It's easy to make this patch again: > > 1) run mercurial's contrib/bytify_strings.py script with the -- >dictiter argument > > 2) fix up the few calls to exthelper.addattr to _not_ use byte strings >(because those strings are passed to python's addattr() builtin, and >that wants unicode). > > That's it! > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve-ext] py3: broad pass for python3 compatibility
> Does this make any existing tests for evolve pass on py3? The evolve extension wasn't even loading in py3 before this patch -- mercurial core would reject it for trying to add unicode strings to the command table. > Taking other patches first would mean this patch would be hard to > apply and it would miss any newly added code, so the question is: how > hard would it be to create this patch again? It's easy to make this patch again: 1) run mercurial's contrib/bytify_strings.py script with the -- dictiter argument 2) fix up the few calls to exthelper.addattr to _not_ use byte strings (because those strings are passed to python's addattr() builtin, and that wants unicode). That's it! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH v3] py3: properly reject non-encoded strings given to hgweb
# HG changeset patch # User Ludovic Chabant # Date 1555683992 0 # Fri Apr 19 14:26:32 2019 + # Branch stable # Node ID 6805ebb0f1f6a7f34a74732ee6f6ec14e8824e42 # Parent 3611368a1af3037427eb59635c7dad8dab67c606 py3: properly reject non-encoded strings given to hgweb diff --git a/mercurial/hgweb/__init__.py b/mercurial/hgweb/__init__.py --- a/mercurial/hgweb/__init__.py +++ b/mercurial/hgweb/__init__.py @@ -13,6 +13,7 @@ from ..i18n import _ from .. import ( +encoding, error, pycompat, ) @@ -38,6 +39,9 @@ - list of virtual:real tuples (multi-repo view) ''' +if isinstance(config, pycompat.unicode): +raise error.ProgrammingError( +'Mercurial only supports encoded strings: %r' % config) if ((isinstance(config, bytes) and not os.path.isdir(config)) or isinstance(config, dict) or isinstance(config, list)): # create a multi-dir interface ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH v3] py3: handle meta-path finders that only use pre-python3.4 API
# HG changeset patch # User Ludovic Chabant # Date 1555683918 0 # Fri Apr 19 14:25:18 2019 + # Branch stable # Node ID 3611368a1af3037427eb59635c7dad8dab67c606 # Parent 4a8d9ed864754837a185a642170cde24392f9abf py3: handle meta-path finders that only use pre-python3.4 API diff --git a/mercurial/__init__.py b/mercurial/__init__.py --- a/mercurial/__init__.py +++ b/mercurial/__init__.py @@ -54,7 +54,16 @@ if finder == self: continue -spec = finder.find_spec(fullname, path, target=target) +# Originally the API was a `find_module` method, but it was +# renamed to `find_spec` in python 3.4, with a new `target` +# argument. +find_spec_method = getattr(finder, 'find_spec', None) +if find_spec_method: +spec = find_spec_method(fullname, path, target=target) +else: +spec = finder.find_module(fullname) +if spec is not None: +spec = importlib.util.spec_from_loader(fullname, spec) if spec: break ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2 v2] py3: handle meta-path finders that only use pre-python3.4 API
> Does find_module() return a spec? We might instead have to skip py3.4 > finders. Ah you're right, it returns something else. It looks like Python does this: try: find_spec = finder.find_spec except AttributeError: loader = finder.find_module(name) if loader is None: return None return importlib.util.spec_from_loader(name, loader) else: return find_spec(name) We can either emulate the same logic, or just skip old finders? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] py3: convert unicode paths given for hgweb config
> > If we want to be consistent with other things like the hgclient API, > > maybe we shouldn't be nice and encode the string for the caller, and > > instead reject anything that's not bytes? > > Sounds good to reject unicodes explicitly. I had sent a v2 of my patch with a better way to encode the string, but now that I'm checking the hgclient code I don't think it explicitly rejects unicode strings? I think it just dies very quickly as soon as it tries to concatenate a string and a bytes. Is there some other precedent for rejecting arguments explicitly? Is it just about raising some ValueError exception? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] py3: protect from exceptions thrown by other meta-path finders
> > Gracefully handling missing find_spec() should be fine. But we should > use hasattr() or getattr() for that (Mercurial’s linter may insist on > getattr() because I think we still ban hasattr()) because bare > “except:” is bad. Thanks, I sent a v2 of my patches yesterday evening with that change in them. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 v2] py3: convert unicode paths given for hgweb config
# HG changeset patch # User Ludovic Chabant # Date 1555683992 0 # Fri Apr 19 14:26:32 2019 + # Branch stable # Node ID eaf4d232055361fd79e5d5299fc76d641be2fb6c # Parent 8214209f30a56052124b7f530d2a955a2b93b14f py3: convert unicode paths given for hgweb config diff --git a/mercurial/hgweb/__init__.py b/mercurial/hgweb/__init__.py --- a/mercurial/hgweb/__init__.py +++ b/mercurial/hgweb/__init__.py @@ -13,6 +13,7 @@ from ..i18n import _ from .. import ( +encoding, error, pycompat, ) @@ -38,6 +39,8 @@ - list of virtual:real tuples (multi-repo view) ''' +if isinstance(config, pycompat.unicode): +config = encoding.unitolocal(config) if ((isinstance(config, bytes) and not os.path.isdir(config)) or isinstance(config, dict) or isinstance(config, list)): # create a multi-dir interface ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 v2] py3: handle meta-path finders that only use pre-python3.4 API
# HG changeset patch # User Ludovic Chabant # Date 1555683918 0 # Fri Apr 19 14:25:18 2019 + # Branch stable # Node ID 8214209f30a56052124b7f530d2a955a2b93b14f # Parent 4a8d9ed864754837a185a642170cde24392f9abf py3: handle meta-path finders that only use pre-python3.4 API diff --git a/mercurial/__init__.py b/mercurial/__init__.py --- a/mercurial/__init__.py +++ b/mercurial/__init__.py @@ -54,7 +54,13 @@ if finder == self: continue -spec = finder.find_spec(fullname, path, target=target) +# Originally the API was a `find_module` method, but it was +# renamed to `find_spec` in python 3.4, with a new `target` +# argument. +if hasattr(finder, 'find_spec'): +spec = finder.find_spec(fullname, path, target=target) +else: +spec = finder.find_module(fullname, path) if spec: break ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] py3: convert unicode paths given for hgweb config
> We'll probably need to fix the caller to not pass in a unicode. Can > you copy-paste the traceback? The caller is my own code. It's basically a custom version of the hgweb.wsgi file (see https://www.mercurial-scm.org/repo/hg/file/tip/contrib/hgweb.wsgi). If we want to be consistent with other things like the hgclient API, maybe we shouldn't be nice and encode the string for the caller, and instead reject anything that's not bytes? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] py3: protect from exceptions thrown by other meta-path finders
> > Is there any list of exceptions that are known to be safely > suppressed? Catching AttributeError, TypeError, etc. seems bad. > In my case, it's just a missing method so AttributeError is what's being raised I think (see https://github.com/benjaminp/six/blob/master/six.py#L164). My rationale here was that there could be any 3rd party finder in there, so it might make sense to protect Mercurial from crashing because of exceptions thrown from other packages. However, now that I think about it, another maybe more correct way to fix this specific bug might be to check for `find_spec` and, if missing, call `find_module` instead. As per the Python docs (see https://docs.python.org/3/library/importlib.html#importlib.abc.MetaPathFinder), the latter was deprecated in 3.4, the former is new in 3.4. So my guess is that _SixMetaPathImporter uses the old API so that it's compatible with all versions of Python 3 (as Python probably does this fallback on the old API if the new doesn't exist). Would that be better? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2] py3: protect from exceptions thrown by other meta-path finders
# HG changeset patch # User Ludovic Chabant # Date 1555683918 0 # Fri Apr 19 14:25:18 2019 + # Branch stable # Node ID 96a51193678400abf9d04af65e60a8dccf540cd7 # Parent 4a8d9ed864754837a185a642170cde24392f9abf py3: protect from exceptions thrown by other meta-path finders diff --git a/mercurial/__init__.py b/mercurial/__init__.py --- a/mercurial/__init__.py +++ b/mercurial/__init__.py @@ -54,9 +54,14 @@ if finder == self: continue -spec = finder.find_spec(fullname, path, target=target) -if spec: -break +try: +spec = finder.find_spec(fullname, path, target=target) +if spec: +break +except: +# Some finders don't implement `find_spec`, like +# `_SixMetaPathImporter`. +pass # This is a Mercurial-related module but we couldn't find it # using the previously-registered finders. This likely means ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2] py3: convert unicode paths given for hgweb config
# HG changeset patch # User Ludovic Chabant # Date 1555683992 0 # Fri Apr 19 14:26:32 2019 + # Branch stable # Node ID b1baf28288faf3fd628d5104211e1482e77bdf39 # Parent 96a51193678400abf9d04af65e60a8dccf540cd7 py3: convert unicode paths given for hgweb config diff --git a/mercurial/hgweb/__init__.py b/mercurial/hgweb/__init__.py --- a/mercurial/hgweb/__init__.py +++ b/mercurial/hgweb/__init__.py @@ -38,6 +38,8 @@ - list of virtual:real tuples (multi-repo view) ''' +if isinstance(config, str): +config = config.encode() if ((isinstance(config, bytes) and not os.path.isdir(config)) or isinstance(config, dict) or isinstance(config, list)): # create a multi-dir interface ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Mercurial docs improvements?
> On Feb 18, 2019, at 15:30, Jordi Gutiérrez Hermoso > wrote: > > There's an important dogfooding component here. If we think that the > hgweb docs are useless, then we should completely get rid of them and > just point people instead to the new docs you're building. I don’t believe the two should necessarily be the exact same thing — hgweb shipping with documentation support is good for people who run hgweb, especially because in some cases (like extensions help) it gives them *contextual* documentation. But the official website is a bit different. At the end of the day, >95% of the documentation will come from docstrings or help topics that are in the codebase, so the _source_ is the same between hgweb and the website, and that's what matters most, I think. Whether the Mercurial website is running hgweb to render the help pages from those docstrings, or is running some bit of Flask code that reads the same docstrings, or is just serving static http files that were baked offline out of those docstrings... that's an implementation detail. What matters is that people write command and topic help text once and it shows up everywhere. Now the question, to me, is what works best for the specific case that is the Mercurial website. In my mind, offline-baked static http files works best because they're performant and make it easy to handle versioning. And it sounds a lot easier to do than having to modify hgweb to have some "global" (non-repo-contextual) help section, figure a way to map it to a URL that makes sense[1], and figure out how to handle versioning (we're not going to run one hgweb instance per version of Mercurial)... unless there's some easy way to bend hgweb to our will? [1]: my guess is that the hgweb server is mapped to mercurial- scm.org/repo so this would mean the documentation URLs would have to start with "repo/" and the OCD in me is quite uncomfortable with that :) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Mercurial docs improvements?
> > - This would let us see the doc for, say, `hg status` over at > > https://www.mercurial-scm.org/doc/hg-status.1.html > > As we discussed in IRC, that page already exists, but it's hard > to find: > > https://www.mercurial-scm.org/repo/hg/help/status Ah yes I forgot to mention that. There's a couple of things that I don't like about this, though: 1) It doesn't work well (or at all) if we are thinking of having access to versioned docs. 2) I don't really like the idea of having an official Mercurial website that's an unholy mix of Flask views, generated html pages, wiki pages, and hgweb-served pages. We can try to make it look consistent (and thus transparent) by sharing templates and CSS styles, though, but, well, my gut feeling is that we should try and have a few moving parts as possible. 3) That URL is for the specific "status" command help of the "hg" repository. You can access the same page here for the "hg-committed" repository: https://www.mercurial-scm.org/repo/hg-committed/help/status So this duplicates things and makes things confusing for users, has a non- obvious URL, and, for pages like the "extensions" help (https://www.mercurial-scm.org/repo/hg-committed/help/extensions) lists the enabled/disabled extensions *for that repo*. So as such, I don't think it's a good fit for a global, repo-independent reference documentation. > We can make the hgweb installation at mercurial-scm.org use whatever > template we want. It makes sense to me to give it a template that > matches the non-hgweb part of mercurial-scm.org, including static > links back to the non-hgweb site. I think making a custom template > might be easier than writing parsers and converters into manpage > formats (I've never really liked manpage formats to begin with though, > so maybe my opinion here doesn't count.) Making a prettier default template for hgweb (and one that would be consistent with the official website) would definitely be awesome indeed, but it's outside the scope of what I want to achieve here. (as such, for now at least, I'm going to ignore all your other comments about hgweb :) ) I don't think we need to write parser and converters either. The hg help pages can be generated into rST and that's probably all we need for now. Although I could of course be mistaken, I frankly anticipate spending a lot more time fiddling around with CSS, templates, and Makefiles that I would with writing extra bits of code for HTML rendering or whatnot. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Mercurial docs improvements?
Hi there!
I started playing around with improving the Mercurial docs but before I
work on it more and send some patches, I wanted to get some opinions
about what people would like to see here. Sorry about the length of this
email but there's a lot to do IMHO.
What I have in mind is the following grand plan (I don't know if I'll
get to do everything but this is what I would _like_ to happen):
1. Make doc/Makefile also generate one html and man page per command and
extension.
- This would let us see the doc for, say, `hg status` over at
https://www.mercurial-scm.org/doc/hg-status.1.html
- When you just want to check the docs for a single command, it's a lot
easier than having to search "status" (with multiple hits) on the
giant hg.1.html page (I know I could just check the doc in my
terminal but sometimes I can't, or I prefer the browser for one
reason or another)
- It would generate `hg-.1` pages (note the .1 suffix) because we
have this 1:1 mapping between man pages and html pages -- I assume we
want to keep that.
- This would also give us the ability to type, say, `man hg-status` and
get a man-formatted version of `hg help status`. I don't care too much
for that but I suppose it doesn't hurt.
2. Improve integration of generated docs inside the website
- Change the html template used to generate the html docs so that it has
the standard website navigation around the content.
- Right now that's the top horizontal menu, more or less (but that might
change, see next points).
- If people prefer the online docs to not have this, we could generate 2
versions of the html: the `.1.html` would be the "naked" man-
like doc in html form, while the `.html` (without the man-
like number suffix) would be the "website integrated form"... or maybe
not and we just force people to hit the Back button often.
- Generate some doc index page with links to all the other generated
pages (categorized links to each command page, general links to
hgignore/hgrc, extensions, etc.)
3. Improve the website itself
- Adding some proper link to the reference documentation (i.e. the
generated html doc pages... note how my previous point mentions
generating an index page for those, so that's what we would link to)
- Take a hard look at the navigation. Right now we have only a top
horizontal menu, which means only a handful of menu items at most.
This means that a lot of the navigation should be done with clear
lists of subpages on each top level page, so that it's easy to get a
mental picture of the website's structure, but in practice the links
are placed organically in the text and I find that confusing. It's
especially confusing when you have somewhat duplicate things (a
"Quickstart" sidebar on the main page, but also a dedicated
"quickstart" page), links to take you out to the wiki, interesting
pages whose links are buried in the text ("learn" page), etc.
- There should be a "Release Notes" or "What's New" link on the website
(I couldn't find one?), shown prominently on the "Downloads" page, and
that should link to a "proper" website page, too (possibly generated
like the reference docs), instead of linking to the wiki (but that
depends on how much manual labour goes into publishing the release
notes, I'm not familiar with that process).
- Similarly, I think the "Extensions" page on the website should be a
"proper" website page, at least as far as general docs and bundled
extensions go. It could link to the wiki for 3rd party extensions.
Honestly, it could (should?) be a link to the `hg help extensions`
topic as generated in html form, with some added bells and whistles
like links to each extension's reference doc page.
- Add some "Get Involved" page (couldn't find any either?) for how to
ask for help, report bugs, and contribute patches.
4. Make reference docs versioned
- The reference docs (hg.1.html, hg-status.1.html, hgignore.5.html,
etc.) should be versioned, so you can check out the docs from a
prior version of Mercurial (which is particularly helpful when
helping someone).
Phew, I think that's it! Right now, I've got #1 mostly figured out and
done, and I started playing around with #2.
Comments?
--
l u d o .
. 8 0 17 80
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] extdiff: support tools that can be run simultaneously
> > Oh right, that's another thing I wanted to ask -- how would I test that?
> > The only idea I have is to log some verbose message ("tool %s has a
> > graphical interface, launching processes simultaneously") and detect that
> > in test. Is there any better way?
>
> Something like that. Maybe you can use 'sleep x; echo y' (x/y depending on
> e.g.
> filename) as a merge tool to check if the tool spawns asynchronously or not,
> if that improves the test coverage.
I opted for the verbose trace -- I figured I didn't want to contribute to
making the test suite run noticeably longer, and the tricky code to test is
figuring out if we should spawn processes asynchronously or not -- not the code
that actually does the spawning.
--
l u d o .
. 8 0 17 80
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V2] extdiff: support tools that can be run simultaneously
# HG changeset patch
# User Ludovic Chabant
# Date 1549173529 28800
# Sat Feb 02 21:58:49 2019 -0800
# Node ID b8e97fbea8490173387735e72cc424c21d7a1c04
# Parent 3a3b053d0882a33ba7ea667052e445b193ffa4df
extdiff: support tools that can be run simultaneously
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -59,6 +59,22 @@
[diff-tools]
kdiff3.diffargs=--L1 '$plabel1' --L2 '$clabel' $parent $child
+If a program has a graphical interface, it might be interesting to tell
+Mercurial about it. It will prevent the program from being mistakenly
+used in a terminal-only environment (such as an SSH terminal session),
+and will make :hg:`extdiff --per-file` open multiple file diffs at once
+instead of one by one (if you still want to open file diffs one by one,
+you can use the --confirm option).
+
+Declaring that a tool has a graphical interface can be done with the
+``gui`` flag next to where ``diffargs`` are specified:
+
+::
+
+ [diff-tools]
+ kdiff3.diffargs=--L1 '$plabel1' --L2 '$clabel' $parent $child
+ kdiff3.gui = true
+
You can use -I/-X and list of file or directory names like normal
:hg:`diff` command. The extdiff extension makes snapshots of only
needed files, so running the external diff program will actually be
@@ -71,6 +87,7 @@
import re
import shutil
import stat
+import subprocess
from mercurial.i18n import _
from mercurial.node import (
@@ -105,11 +122,19 @@
generic=True,
)
+configitem('extdiff', br'gui\..*',
+generic=True,
+)
+
configitem('diff-tools', br'.*\.diffargs$',
default=None,
generic=True,
)
+configitem('diff-tools', br'.*\.gui$',
+generic=True,
+)
+
# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core'
for
# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
# be specifying the version(s) of Mercurial they are tested with, or
@@ -176,13 +201,26 @@
cmdline += ' $parent1 $child'
return re.sub(regex, quote, cmdline)
-def _runperfilediff(cmdline, repo_root, ui, do3way, confirm,
+def _systembackground(cmd, environ=None, cwd=None):
+''' like 'procutil.system', but returns the Popen object directly
+so we don't have to wait on it.
+'''
+cmd = procutil.quotecommand(cmd)
+env = procutil.shellenviron(environ)
+proc = subprocess.Popen(procutil.tonativestr(cmd),
+shell=True, close_fds=procutil.closefds,
+env=procutil.tonativeenv(env),
+cwd=pycompat.rapply(procutil.tonativestr, cwd))
+return proc
+
+def _runperfilediff(cmdline, repo_root, ui, guitool, do3way, confirm,
commonfiles, tmproot, dir1a, dir1b,
dir2root, dir2,
rev1a, rev1b, rev2):
# Note that we need to sort the list of files because it was
# built in an "unstable" way and it's annoying to get files in a
# random order, especially when "confirm" mode is enabled.
+waitprocs = []
totalfiles = len(commonfiles)
for idx, commonfile in enumerate(sorted(commonfiles)):
path1a = os.path.join(tmproot, dir1a, commonfile)
@@ -228,14 +266,32 @@
parent1=path1a, plabel1=label1a,
parent2=path1b, plabel2=label1b,
child=path2, clabel=label2)
-ui.debug('running %r in %s\n' % (pycompat.bytestr(curcmdline),
- tmproot))
-# Run the comparison program and wait for it to exit
-# before we show the next file.
-ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+if confirm or not guitool:
+# Run the comparison program and wait for it to exit
+# before we show the next file.
+# This is because either we need to wait for confirmation
+# from the user between each invocation, or because, as far
+# as we know, the tool doesn't have a GUI, in which case
+# we can't run multiple CLI programs at the same time.
+ui.debug('running %r in %s\n' %
+ (pycompat.bytestr(curcmdline), tmproot))
+ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+else:
+# Run the comparison program but don't wait, as we're
+# going to rapid-fire each file diff and then wait on
+# the whole group.
+ui.debug('running %r in %s (backgrounded)\n' %
+ (pycompat.bytestr(curcmdline), tmproot))
+proc = _systembackground(curcmdline, cwd=tmproot)
+waitprocs.append(proc)
-def dodiff(ui, repo, cmdline, pats, opts):
+if waitproc
Re: [PATCH 1 of 2] extdiff: support tools that can be run simultaneously
> Generally looks good. Can you add some tests?
Oh right, that's another thing I wanted to ask -- how would I test that? The
only idea I have is to log some verbose message ("tool %s has a graphical
interface, launching processes simultaneously") and detect that in test. Is
there any better way?
--
l u d o .
. 8 0 17 80
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] extdiff: support tools that can be run simultaneously
One thing I'm not quite sure about this change is how to handle the new "gui" config flag. I wanted to use (abuse?) the difference between False and None in order to figure out if we found the flag in a section or not (and fall back to another section if not). However, passing None as the default to ui.config yields warnings about mismatched default values, which is why I ended up registering the flag with None as its default... that might be wrong, but I don't know if there's a better way, short of doing some clunky juggling with ui.hasconfig ? (I also just realized I should use ui.configbool instead of ui.config) Unrelated: git email reviews allow editing a patch before sending it via email, so you can add some review-only text in it (look for "timely commentary" on this page: https://git-scm.com/docs/git-format-patch#_discussion). Is there some similar convention for mercurial patch emails? Or are we supposed to just reply to the patch email right away like I'm doing right now? Thanks! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2] extdiff: support tools that can be run simultaneously
# HG changeset patch
# User Ludovic Chabant
# Date 1549173529 28800
# Sat Feb 02 21:58:49 2019 -0800
# Node ID b08ea934c2d5ac097b171ca74e826e4f9dea86a9
# Parent 3a3b053d0882a33ba7ea667052e445b193ffa4df
extdiff: support tools that can be run simultaneously
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -59,6 +59,22 @@
[diff-tools]
kdiff3.diffargs=--L1 '$plabel1' --L2 '$clabel' $parent $child
+If a program has a graphical interface, it might be interesting to tell
+Mercurial about it. It will prevent the program from being mistakenly
+used in a terminal-only environment (such as an SSH terminal session),
+and will make :hg:`extdiff --per-file` open multiple file diffs at once
+instead of one by one (if you still want to open file diffs one by one,
+you can use the --confirm option).
+
+Declaring that a tool has a graphical interface can be done with the
+``gui`` flag next to where ``diffargs`` are specified:
+
+::
+
+ [diff-tools]
+ kdiff3.diffargs=--L1 '$plabel1' --L2 '$clabel' $parent $child
+ kdiff3.gui = true
+
You can use -I/-X and list of file or directory names like normal
:hg:`diff` command. The extdiff extension makes snapshots of only
needed files, so running the external diff program will actually be
@@ -71,6 +87,7 @@
import re
import shutil
import stat
+import subprocess
from mercurial.i18n import _
from mercurial.node import (
@@ -105,11 +122,19 @@
generic=True,
)
+configitem('extdiff', br'gui\..*',
+generic=True,
+)
+
configitem('diff-tools', br'.*\.diffargs$',
default=None,
generic=True,
)
+configitem('diff-tools', br'.*\.gui$',
+generic=True,
+)
+
# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core'
for
# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
# be specifying the version(s) of Mercurial they are tested with, or
@@ -176,13 +201,26 @@
cmdline += ' $parent1 $child'
return re.sub(regex, quote, cmdline)
-def _runperfilediff(cmdline, repo_root, ui, do3way, confirm,
+def _systemdetached(cmd, environ=None, cwd=None):
+''' like 'procutil.system', but returns the Popen object directly
+so we don't have to wait on it.
+'''
+cmd = procutil.quotecommand(cmd)
+env = procutil.shellenviron(environ)
+proc = subprocess.Popen(procutil.tonativestr(cmd),
+shell=True, close_fds=procutil.closefds,
+env=procutil.tonativeenv(env),
+cwd=pycompat.rapply(procutil.tonativestr, cwd))
+return proc
+
+def _runperfilediff(cmdline, repo_root, ui, guitool, do3way, confirm,
commonfiles, tmproot, dir1a, dir1b,
dir2root, dir2,
rev1a, rev1b, rev2):
# Note that we need to sort the list of files because it was
# built in an "unstable" way and it's annoying to get files in a
# random order, especially when "confirm" mode is enabled.
+waitprocs = []
totalfiles = len(commonfiles)
for idx, commonfile in enumerate(sorted(commonfiles)):
path1a = os.path.join(tmproot, dir1a, commonfile)
@@ -231,11 +269,27 @@
ui.debug('running %r in %s\n' % (pycompat.bytestr(curcmdline),
tmproot))
-# Run the comparison program and wait for it to exit
-# before we show the next file.
-ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+if confirm or not guitool:
+# Run the comparison program and wait for it to exit
+# before we show the next file.
+# This is because either we need to wait for confirmation
+# from the user between each invocation, or because, as far
+# as we know, the tool doesn't have a GUI, in which case
+# we can't run multiple CLI programs at the same time.
+ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+else:
+# Run the comparison program but don't wait, as we're
+# going to rapid-fire each file diff and then wait on
+# the whole group.
+proc = _systemdetached(curcmdline, cwd=tmproot)
+waitprocs.append(proc)
-def dodiff(ui, repo, cmdline, pats, opts):
+if waitprocs:
+with ui.timeblockedsection('extdiff'):
+for proc in waitprocs:
+proc.wait()
+
+def dodiff(ui, repo, cmdline, pats, opts, guitool=False):
'''Do the actual diff:
- copy to a temp structure if diffing 2 internal revisions
@@ -382,7 +436,8 @@
else:
# Run the external tool once for each pair of fil
[PATCH 2 of 2] extdiff: don't run gui programs when in a cli-only environment
# HG changeset patch
# User Ludovic Chabant
# Date 1549173509 28800
# Sat Feb 02 21:58:29 2019 -0800
# Node ID 75d87a5ac4d0d26e02d21df87d2a89ac59c6d535
# Parent b08ea934c2d5ac097b171ca74e826e4f9dea86a9
extdiff: don't run gui programs when in a cli-only environment
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -547,6 +547,9 @@
self._isgui = isgui
def __call__(self, ui, repo, *pats, **opts):
+if self._isgui and not procutil.gui():
+msg = (_("tool '%s' requires a GUI") % self._cmd)
+raise error.Abort(msg)
opts = pycompat.byteskwargs(opts)
options = ' '.join(map(procutil.shellquote, opts['option']))
if options:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V3] extdiff: add --per-file and --confirm options
> My idea is basically as follows: > > if cmd and opts found in extdiff: > # it must be a user-defined tool > look for extdiff.gui. > elif cmd.diffargs found in diff-tools: > # perhaps it's a stock tool template defined in [diff-tools] (less > common) > look for diff-tools..gui > elif cmd.diffargs found in merge-tools: > # perhaps it's a stock tool template defined in [merge-tools] (common) > look for merge-tools..gui In practice (again, after a bit of testing), I got confused again pretty easily. For instance: [extdiff] blah = gui.blah = True ...doesn't do what I would expect. If "blah" is *not* a stock tool, and is in my $PATH, it would default to calling "blah $parent1 $child", but the GUI flag would be False because we didn't find any diff options per se. Alternatively, if "blah" *is* one of the stock tools, I can do: [extdiff] blah = /path/to/blah gui.blah = True This would override the path from the stock tools (while still using the stock tool arguments), but wouldn't override the GUI flag, which seems inconsistent. The same problem (inconsistent overriding) happens if I use "cmd.blah" instead of "blah". So far, something that feels vaguely intuitive to me would be: (1) get "gui." from [extdiff] (2) if we have to check [diff-tools] and/or [merge-tools] to find arguments: if we didn't find any setting at step (1): check ".gui" in the same section we found the arguments (3) fall back to False if nothing is found. Comments? > Nice. I think the core version can also be "hg extdiff" that supports > -t/--tool > option. And the extdiff extension will wrap the core extdiff command to > support > -p/--program and command aliases for backward compatibility. Interesting -- that approach could indeed make it easier to handle. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V4] extdiff: add --per-file and --confirm options
> Unused in this version. Removed. > > Looks like unrelated change. Also removed. Indeed, sorry, I didn't correctly split my commit in 2. Thanks for the cleanup! -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V3] extdiff: add --per-file and --confirm options
> My point was that it could be addressed by checking the gui flag (or adding > new dedicated flag to [diff/merge-tools].) It's a tool property whether > multiple processes can be spawned or not. After testing a bit, it turns out it's actually a bit more complicated than I anticipated. Consider this: [extdiff] blah = [diff-tools] blah.gui = True [merge-tools] blah.diffargs = -a -b -c The extdiff command will find the options to use on the command line from the [merge-tools] sections. Does this mean we ignore the "gui = True" from [diff-tools] even though that's clearly intended for *diff commands to use? It could also be the opposite, where the "diffargs" are found in [diff-tools] and "gui" is set in [merge-tools]. And there's the case where no "diffargs" are provided anywhere, in which case the "gui" setting might be taken from wherever we find, leading to the problem where, if you have: [extdiff] blah = [merge-tools] blah.gui = True ...then it will consider "blah" to be a GUI tool... until you add a [diff-tools] section with "blah.diffargs" and suddenly "blah" isn't considered a GUI anymore because the presence of a "diffargs" setting there made us stop looking at the [merge-tools] section. I think the best we can do might be to look for "gui" in [merge-tools] _only_ if "diffargs" is also there, and otherwise _only_ look for "gui" in [diff-tools]... ideas? Unless I'm missing something, I feels like we're basically reaching the limits of the, ahem, rather organic spec of the extdiff command (there's not even a help topic on "config.diff-tools"). After we solve this, I would be totally ok working on some patches to make a core "hg difftool" command or something, with a more strict UX. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V3] extdiff: add --per-file and --confirm options
> Can you split this to a separate patch? It'll need a bit more churn, > and we wouldn't want to make it a blocker of the --per-file/--confirm > feature. I just sent PATCH V4 which should be just that feature alone, where, if you don't specify --confirm, it launches the external tool one by one. The second patch should be ready soon. Thanks! -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V4] extdiff: add --per-file and --confirm options
# HG changeset patch
# User Ludovic Chabant
# Date 1548831555 28800
# Tue Jan 29 22:59:15 2019 -0800
# Node ID 7127fdab8807f3a3bcf2b1eebb1ee4084233f21f
# Parent ef0e2f7224358c32b0f62b13e83e89ba2399c8cf
extdiff: add --per-file and --confirm options
The new options lets the user control how the external program is run.
By default, Mercurial passes the 2 snapshot directories as usual, but
it can also run the program repeatedly on each file's snapshot pair,
and optionally prompt the user each time.
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -71,6 +71,7 @@
import re
import shutil
import stat
+import subprocess
from mercurial.i18n import _
from mercurial.node import (
@@ -80,6 +81,7 @@
from mercurial import (
archival,
cmdutil,
+encoding,
error,
filemerge,
formatter,
@@ -175,6 +177,66 @@
cmdline += ' $parent1 $child'
return re.sub(regex, quote, cmdline)
+def _runperfilediff(cmdline, repo_root, ui, do3way, confirm,
+commonfiles, tmproot, dir1a, dir1b,
+dir2root, dir2,
+rev1a, rev1b, rev2):
+# Note that we need to sort the list of files because it was
+# built in an "unstable" way and it's annoying to get files in a
+# random order, especially when "confirm" mode is enabled.
+totalfiles = len(commonfiles)
+for idx, commonfile in enumerate(sorted(commonfiles)):
+path1a = os.path.join(tmproot, dir1a, commonfile)
+label1a = commonfile + rev1a
+if not os.path.isfile(path1a):
+path1a = os.devnull
+
+path1b = ''
+label1b = ''
+if do3way:
+path1b = os.path.join(tmproot, dir1b, commonfile)
+label1b = commonfile + rev1b
+if not os.path.isfile(path1b):
+path1b = os.devnull
+
+path2 = os.path.join(dir2root, dir2, commonfile)
+label2 = commonfile + rev2
+
+if confirm:
+# Prompt before showing this diff
+difffiles = _('diff %s (%d of %d)') % (commonfile, idx + 1,
+ totalfiles)
+responses = _('[Yns?]'
+ '$$ &Yes, show diff'
+ '$$ &No, skip this diff'
+ '$$ &Skip remaining diffs'
+ '$$ &? (display help)')
+r = ui.promptchoice('%s %s' % (difffiles, responses))
+if r == 3: # ?
+while r == 3:
+for c, t in ui.extractchoices(responses)[1]:
+ui.write('%s - %s\n' % (c, encoding.lower(t)))
+r = ui.promptchoice('%s %s' %
+(difffiles, responses))
+if r == 0: # yes
+pass
+elif r == 1: # no
+continue
+elif r == 2: # skip
+break
+
+curcmdline = formatcmdline(
+cmdline, repo_root, do3way=do3way,
+parent1=path1a, plabel1=label1a,
+parent2=path1b, plabel2=label1b,
+child=path2, clabel=label2)
+ui.debug('running %r in %s\n' % (pycompat.bytestr(curcmdline),
+ tmproot))
+
+# Run the comparison program and wait for it to exit
+# before we show the next file.
+ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+
def dodiff(ui, repo, cmdline, pats, opts):
'''Do the actual diff:
@@ -201,6 +263,9 @@
else:
ctx1b = repo[nullid]
+perfile = opts.get('per_file')
+confirm = opts.get('confirm')
+
node1a = ctx1a.node()
node1b = ctx1b.node()
node2 = ctx2.node()
@@ -217,6 +282,8 @@
if opts.get('patch'):
if subrepos:
raise error.Abort(_('--patch cannot be used with --subrepos'))
+if perfile:
+raise error.Abort(_('--patch cannot be used with --per-file'))
if node2 is None:
raise error.Abort(_('--patch requires two revisions'))
else:
@@ -304,15 +371,23 @@
label1b = None
fnsandstat = []
-# Run the external tool on the 2 temp directories or the patches
-cmdline = formatcmdline(
-cmdline, repo.root, do3way=do3way,
-parent1=dir1a, plabel1=label1a,
-parent2=dir1b, plabel2=label1b,
-child=dir2, clabel=label2)
-ui.debug('running %r in %s\n' % (pycompat.bytestr(cmdline),
- tmproot))
-ui.system(cmdline, cwd=tmproot, blockedtag='extdiff')
+
Re: [PATCH V3] extdiff: add --per-file and --confirm options
> I do understand however that it would be easy for users to get in a > messy situation so I guess I'll add a "--batch" option to "--per-file" > if that's OK? Another way to solve this would be to actually default to "--prompt" when "--per-file" is specified, and require "--no-prompt" if you want the "batch fire processes" mode for tools like Meld or BeyondCompare. It looks like that's how Git does it, and that would save us an option, which I like. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V3] extdiff: add --per-file and --confirm options
> What I pointed out last time was that we shouldn't spawn non-GUI tools > asynchronously. You can see the problem by the following command on Unix. > > $ hg --config extdiff.vimdiff= vimdiff -c. --per-file Mmmh so for this I figured people would use the "--confirm" flag because, if we use the usual (blocking) process launch, there's not much difference anymore (the only difference being that the next vimdiff process launches automatically instead of the user having the press Enter). It would also prevent using this command for programs that support showing multiple diffs, usually in a tabbed way. That's how BeyondCompare works (you can batch-fire multiple processes and it will just keep opening more diff tabs in the same instance). I'm also currently coming up with a script to do this using Vim's clientserver functionality (so you run the hg command and it will open Vim with each file-diff in a tab). I do understand however that it would be easy for users to get in a messy situation so I guess I'll add a "--batch" option to "--per-file" if that's OK? > And perhaps, it's too late to compare ui.config value based on self._cmd > here. If self._cmd is the key of [diff-tools] for example, we shouldn't look > for [merge-tools]. Ah good point, thanks. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V3] extdiff: add --per-file and --confirm options
# HG changeset patch
# User Ludovic Chabant
# Date 1547180577 28800
# Thu Jan 10 20:22:57 2019 -0800
# Node ID 5686d56d5a2cebaa504383b270e359156352090f
# Parent ef0e2f7224358c32b0f62b13e83e89ba2399c8cf
extdiff: add --per-file and --confirm options
The new options lets the user control how the external program is run.
By default, Mercurial passes the 2 snapshot directories as usual, but
it can also run the program repeatedly on each file's snapshot pair,
and optionally prompt the user each time.
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -71,6 +71,7 @@
import re
import shutil
import stat
+import subprocess
from mercurial.i18n import _
from mercurial.node import (
@@ -80,6 +81,7 @@
from mercurial import (
archival,
cmdutil,
+encoding,
error,
filemerge,
formatter,
@@ -175,6 +177,100 @@
cmdline += ' $parent1 $child'
return re.sub(regex, quote, cmdline)
+def _checktool(ui, tool):
+if not procutil.gui():
+needsgui = (ui.config('diff-tools', tool+'.gui') or
+ui.config('merge-tools', tool+'.gui'))
+if needsgui:
+ui.warn(_("tool %s requires a GUI\n") % tool)
+return False
+return True
+
+def _systemdetached(cmd, environ=None, cwd=None):
+''' like 'procutil.system', but returns the Popen object directly
+so we don't have to wait on it.
+'''
+cmd = procutil.quotecommand(cmd)
+env = procutil.shellenviron(environ)
+proc = subprocess.Popen(procutil.tonativestr(cmd),
+shell=True, close_fds=procutil.closefds,
+env=procutil.tonativeenv(env),
+cwd=pycompat.rapply(procutil.tonativestr, cwd))
+return proc
+
+def _runperfilediff(cmdline, repo_root, ui, do3way, confirm,
+commonfiles, tmproot, dir1a, dir1b,
+dir2root, dir2,
+rev1a, rev1b, rev2):
+# Note that we need to sort the list of files because it was
+# built in an "unstable" way and it's annoying to get files in a
+# random order, especially when "confirm" mode is enabled.
+waitprocs = []
+totalfiles = len(commonfiles)
+for idx, commonfile in enumerate(sorted(commonfiles)):
+path1a = os.path.join(tmproot, dir1a, commonfile)
+label1a = commonfile + rev1a
+if not os.path.isfile(path1a):
+path1a = os.devnull
+
+path1b = ''
+label1b = ''
+if do3way:
+path1b = os.path.join(tmproot, dir1b, commonfile)
+label1b = commonfile + rev1b
+if not os.path.isfile(path1b):
+path1b = os.devnull
+
+path2 = os.path.join(dir2root, dir2, commonfile)
+label2 = commonfile + rev2
+
+if confirm:
+# Prompt before showing this diff
+difffiles = _('diff %s (%d of %d)') % (commonfile, idx + 1,
+ totalfiles)
+responses = _('[Yns?]'
+ '$$ &Yes, show diff'
+ '$$ &No, skip this diff'
+ '$$ &Skip remaining diffs'
+ '$$ &? (display help)')
+r = ui.promptchoice('%s %s' % (difffiles, responses))
+if r == 3: # ?
+while r == 3:
+for c, t in ui.extractchoices(responses)[1]:
+ui.write('%s - %s\n' % (c, encoding.lower(t)))
+r = ui.promptchoice('%s %s' %
+(difffiles, responses))
+if r == 0: # yes
+pass
+elif r == 1: # no
+continue
+elif r == 2: # skip
+break
+
+curcmdline = formatcmdline(
+cmdline, repo_root, do3way=do3way,
+parent1=path1a, plabel1=label1a,
+parent2=path1b, plabel2=label1b,
+child=path2, clabel=label2)
+ui.debug('running %r in %s\n' % (pycompat.bytestr(curcmdline),
+ tmproot))
+
+if confirm:
+# Run the comparison program and wait for it to exit
+# before we show the next file.
+ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+else:
+# Run the comparison program but don't wait, as we're
+# going to rapid-fire each file diff and then wait on
+# the whole group.
+proc = _systemdetached(curcmdline, cwd=tmproot)
+waitprocs.append(proc)
+
+if waitprocs:
+with ui.
Re: [PATCH 2 of 2 V2] extdiff: add --mode option
> I was just speaking about the current implementation (without your patch.) > Maybe I should say .gui flag isn't checked because the current extdiff never > spawns more than one processes at a time. Yeah, and I meant that I don't understand what's the difference between one vs. multiple processes? > AFAIK, it duplicates alias functionality because there wasn't no [alias] > when the extdiff extension was introduced. And I think that's the major > reason why the extdiff is still an extension. It can't be trivially ported > to a core command. > > If the extdiff had the option to look for a diff tool from stock templates > (i.e. [diff-tools], [merge-tools], and maybe [extdiff]), alias could be > expressed as follows: > > [alias] > bcomp = extdiff --tool bcomp --per-file > > This should be good enough. Yeah makes sense, I figured extdiff predated alias. And I would indeed like if we could clean up the way extdiff integrates with the rest of mercurial... it's not far off since, right now, "hg extdiff -p blah" will just run "blah" instead of first checking if there's a "merge-tools.blah" config... but I guess implementing that would qualify as a breaking change and wouldn't be acceptable? > You can turn off the --per-file by --no-per-file. A separate --dir option > isn't needed as long as --no-per-file == --dir. See hg help flags. Ah right I forgot about the "no" flags... it still feels a bit awkward to use IMHO (since you have to remember the implementation of the alias or extdiff in order to remember to cancel a specific flag) but I can live with that. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 V2] extdiff: add --mode option
> A separate patch seems fine. To make clear, there's no need to check if the > tool is console-based or not unless multiple diff processes are spawned > simultaneously. Currently the .gui flag is tested only by filemerge.py. So you mean I don't need to modify the previous code path (dir-diff), nor do I need to check the gui flag if the "confirm" flag is given (i.e. one external process at a time)? How come? Wouldn't even one gui process be a problem if you're in a shell-only instance of hg? > The latter looks worse for me. Instead, you can use [alias] to pass in > arguments to hg commands. That's true, but then the entire extdiff configuration section would be deprecated if that was the case, no? Like, AFAIK there's not much difference between: [extdiff] cmd.bcomp = /path/to/bcomp opts.bcomp = --whatever and: [alias] bcomp = extdiff -p /path/to/bcomp -o "--whatever" I assume the point of extdiff is to be a slighly better version of an alias for the purpose of a diffing stuff, but maybe someone with a better knowledge of the history of both features can correct me. More importantly, I actually just realized (maybe) why a --mode option might be better. Remember how I intended to have, say, BeyondCompare setup to do per-file-diffs by default, and I would pass a dir-diff option for the 5% cases where I want to only diff a few files in a revision? Well, if we have multiple flags, the extdiff/alias/whatever section of my .hgrc would specify --per-file (so I get per-file diffs by default), but if I then pass --dir (to revert back to a dir-diff), it wouldn't override the default... the extdiff command would instead get both --per-file and --dir, and would most likely throw an error because it doesn't make sense to pass those options togethers. However, with a --mode option, the one I pass on the command line would correctly override the one specified in the .hgrc, and everything works as you would expect... does that make sense? -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 V2] extdiff: add --mode option
Thanks for the comments! > Maybe need to check the .gui flag of the tool. Otherwise the console > would be messed up if the tool wasn't a GUI program. Good point, although the existing extdiff code doesn't seem to be checking that, no? So should I do that in a separate patch? > Well, I think it will produce 8 modes, > dir-or-file x revpair-or-revrage x prompt-or-not. > > And we can even get rid of the dir-or-file mode from command flags by > registering two tools, one for dir-diff and the other for per-file-diff. > I don't know if that is a better idea, but we'll never use the per-file > mode if the tool had a decent support for directory diff. So it's a tool- > specific property at some level. It's not super important, but I disagree about the second-to-last-point here, since I actually wrote this patch with the intent of using per-file diffs with tools like BeyondCompare and vim+DirDiff, which both handle dir diffs pretty well. It's just that 95% of the time I want to see the diff of _all_ the files in a revision, and going through a dir-diff UI just adds unnecessary actions/clicks to get all those diffs to open. The only times I want a dir-diff where I'm going to only look at _some_ of the files is when diffing a big merge. As a result, I was indeed planning on configuring per-file-diffing by default in my .hgrc for those tools, while occasionally passing a dir-diff argument when appropriate. Sure, I could make 2 different tools in my .hgrc if we didn't want to expose new options to the CLI, but for some reason it feels wrong to force users to edit their .hgrc for that. I see your point about modes vs flags. Like I said, I don't have very strong opinions about it so I guess I'll change the second patch and use some flags instead. Are we going for --per-file and --confirm then? Note that the tool config in .hgrc would look a bit less elegant as a result, like this: cmd.blah=/path/to/blah opts.blah=--some-option -z per-file.blah=true confirm.blah=true So instead I'm proposing a new config that specifies options for the hg extdiff command itself, like this: cmd.blah=/path/to/blah opts.blah=--some-option -z hgopts.blah=--per-file --confirm ...but I don't know if that opens up potential abuse or something. What do you think? > A prompt could be shown for the directory diff. I don't think it's useful > right now, but if we had a per-revision-diff mode, it would make some sense > to show prompt to skip some revisions. I would personally just ignore the prompt when showing only one revision in dir-diff mode, but yes, for a revision range dir-diff, it would definitely show a prompt between each revision. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] extdiff: add --mode option
> First, can you break this into a series of a few patches? As you might have seen, I sent the change split in 2 patches to the mailing list. I didn't change anything else yet, so we can keep discussing the CLI on the new patch. Thanks! -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 V2] extdiff: add --mode option
# HG changeset patch
# User Ludovic Chabant
# Date 1547180577 28800
# Thu Jan 10 20:22:57 2019 -0800
# Node ID dfda7867497e5609a03727508dc0da2cab3218a3
# Parent ef0e2f7224358c32b0f62b13e83e89ba2399c8cf
extdiff: add --mode option
The new option lets the user control how the external program is run.
By default, Mercurial passes the 2 snapshot directories as usual, but
it can also run the program repeatedly on each file's snapshot pair,
and optionally prompt the user each time.
The option can be passed via the CLI or be set in the 'extdiff' tool
config.
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -71,6 +71,7 @@
import re
import shutil
import stat
+import subprocess
from mercurial.i18n import _
from mercurial.node import (
@@ -80,6 +81,7 @@
from mercurial import (
archival,
cmdutil,
+encoding,
error,
filemerge,
formatter,
@@ -104,6 +106,11 @@
generic=True,
)
+configitem('extdiff', br'mode\..*',
+default='',
+generic=True,
+)
+
configitem('diff-tools', br'.*\.diffargs$',
default=None,
generic=True,
@@ -175,6 +182,91 @@
cmdline += ' $parent1 $child'
return re.sub(regex, quote, cmdline)
+def _systemdetached(cmd, environ=None, cwd=None):
+''' like 'procutil.system', but returns the Popen object directly
+so we don't have to wait on it.
+'''
+cmd = procutil.quotecommand(cmd)
+env = procutil.shellenviron(environ)
+proc = subprocess.Popen(procutil.tonativestr(cmd),
+shell=True, close_fds=procutil.closefds,
+env=procutil.tonativeenv(env),
+cwd=pycompat.rapply(procutil.tonativestr, cwd))
+return proc
+
+def _runperfilediff(cmdline, repo_root, ui, do3way, diffmode,
+commonfiles, tmproot, dir1a, dir1b,
+dir2root, dir2,
+rev1a, rev1b, rev2):
+# Note that we need to sort the list of files because it was
+# built in an "unstable" way and it's annoying to get files in a
+# random order, especially for the "prompt" mode.
+waitprocs = []
+totalfiles = len(commonfiles)
+for idx, commonfile in enumerate(sorted(commonfiles)):
+path1a = os.path.join(tmproot, dir1a, commonfile)
+label1a = commonfile + rev1a
+if not os.path.isfile(path1a):
+path1a = os.devnull
+
+path1b = ''
+label1b = ''
+if do3way:
+path1b = os.path.join(tmproot, dir1b, commonfile)
+label1b = commonfile + rev1b
+if not os.path.isfile(path1b):
+path1b = os.devnull
+
+path2 = os.path.join(dir2root, dir2, commonfile)
+label2 = commonfile + rev2
+
+if 'p' in diffmode:
+# Prompt before showing this diff
+difffiles = _('diff %s (%d of %d)') % (commonfile, idx + 1,
+ totalfiles)
+responses = _('[Yns?]'
+ '$$ &Yes, show diff'
+ '$$ &No, skip this diff'
+ '$$ &Skip remaining diffs'
+ '$$ &? (display help)')
+r = ui.promptchoice('%s %s' % (difffiles, responses))
+if r == 3: # ?
+while r == 3:
+for c, t in ui.extractchoices(responses)[1]:
+ui.write('%s - %s\n' % (c, encoding.lower(t)))
+r = ui.promptchoice('%s %s' %
+(difffiles, responses))
+if r == 0: # yes
+pass
+elif r == 1: # no
+continue
+elif r == 2: # skip
+break
+
+curcmdline = formatcmdline(
+cmdline, repo_root, do3way=do3way,
+parent1=path1a, plabel1=label1a,
+parent2=path1b, plabel2=label1b,
+child=path2, clabel=label2)
+ui.debug('running %r in %s\n' % (pycompat.bytestr(curcmdline),
+ tmproot))
+
+if 'p' in diffmode:
+# Run the comparison program and wait for it to exit
+# before we show the next file.
+ui.system(curcmdline, cwd=tmproot, blockedtag='extdiff')
+else:
+# Run the comparison program but don't wait, as we're
+# going to rapid-fire each file diff and then wait on
+# the whole group.
+proc = _systemdetached(curcmdline, cwd=tmproot)
+waitprocs.append(proc)
+
+if waitprocs:
+wit
[PATCH 1 of 2 V2] extdiff: move external tool command line building into separate function
# HG changeset patch
# User Ludovic Chabant
# Date 1547180523 28800
# Thu Jan 10 20:22:03 2019 -0800
# Node ID ef0e2f7224358c32b0f62b13e83e89ba2399c8cf
# Parent e546c124217485f54b897d050a9573fc4ab97ab7
extdiff: move external tool command line building into separate function
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -152,6 +152,29 @@
fnsandstat.append((dest, repo.wjoin(fn), os.lstat(dest)))
return dirname, fnsandstat
+def formatcmdline(cmdline, repo_root, do3way,
+ parent1, plabel1, parent2, plabel2, child, clabel):
+# Function to quote file/dir names in the argument string.
+# When not operating in 3-way mode, an empty string is
+# returned for parent2
+replace = {'parent': parent1, 'parent1': parent1, 'parent2': parent2,
+ 'plabel1': plabel1, 'plabel2': plabel2,
+ 'child': child, 'clabel': clabel,
+ 'root': repo_root}
+def quote(match):
+pre = match.group(2)
+key = match.group(3)
+if not do3way and key == 'parent2':
+return pre
+return pre + procutil.shellquote(replace[key])
+
+# Match parent2 first, so 'parent1?' will match both parent1 and parent
+regex = (br'''(['"]?)([^\s'"$]*)'''
+ br'\$(parent2|parent1?|child|plabel1|plabel2|clabel|root)\1')
+if not do3way and not re.search(regex, cmdline):
+cmdline += ' $parent1 $child'
+return re.sub(regex, quote, cmdline)
+
def dodiff(ui, repo, cmdline, pats, opts):
'''Do the actual diff:
@@ -281,28 +304,14 @@
label1b = None
fnsandstat = []
-# Function to quote file/dir names in the argument string.
-# When not operating in 3-way mode, an empty string is
-# returned for parent2
-replace = {'parent': dir1a, 'parent1': dir1a, 'parent2': dir1b,
- 'plabel1': label1a, 'plabel2': label1b,
- 'clabel': label2, 'child': dir2,
- 'root': repo.root}
-def quote(match):
-pre = match.group(2)
-key = match.group(3)
-if not do3way and key == 'parent2':
-return pre
-return pre + procutil.shellquote(replace[key])
-
-# Match parent2 first, so 'parent1?' will match both parent1 and parent
-regex = (br'''(['"]?)([^\s'"$]*)'''
- br'\$(parent2|parent1?|child|plabel1|plabel2|clabel|root)\1')
-if not do3way and not re.search(regex, cmdline):
-cmdline += ' $parent1 $child'
-cmdline = re.sub(regex, quote, cmdline)
-
-ui.debug('running %r in %s\n' % (pycompat.bytestr(cmdline), tmproot))
+# Run the external tool on the 2 temp directories or the patches
+cmdline = formatcmdline(
+cmdline, repo.root, do3way=do3way,
+parent1=dir1a, plabel1=label1a,
+parent2=dir1b, plabel2=label1b,
+child=dir2, clabel=label2)
+ui.debug('running %r in %s\n' % (pycompat.bytestr(cmdline),
+ tmproot))
ui.system(cmdline, cwd=tmproot, blockedtag='extdiff')
for copy_fn, working_fn, st in fnsandstat:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] extdiff: add --mode option
> Sorry for late review. Almost all devs were inactive while holidays. No worries! Between the holidays and the 4.8.2 release, I figured unsolicited new features were a low priority. > First, can you break this into a series of a few patches? > > https://www.mercurial-scm.org/wiki/ContributingChanges#Organizing_patches > > I expect the first patch will probably reorder or extract the current behavior > to function such that new modes can be added. Sure! I wasn't sure exactly what qualifies as multi vs single patch changes. > I don't think "--mode " is good ui. I don't have nice idea, but maybe > it can be split into two flags (e.g. --per-file and --confirm/--interactive)? > > And the config option can specify if the tool support directory diff, for > example. If the tool doesn't support directory diff, and if it's gui, spawn > processes per file by default. I was actually debating with myself between the 2 ways of specifying the diff mode, and eventually I settled on the --mode option. Here's the rationale (although I can be easily convinced to use the alternative): - A --mode option is easier to later extend to more ways to diff things, like for example a mode where, when you pass a revision range, it will diff each revision one by one (invoking the external program N times if there are N revisions in the range). Different flags that may or may not be compatible between each other are more confusing IMHO. - The --confirm option doesn't do anything if you don't also pass --per-file... I don't know if there are other aspects of the hg cli that have this? Now I realize that I also fell short of that downside since "--mode p" doesn't do anything either, but the point is that if we want something to represent "per file" and "per file with prompt", 2 modes do that better than 2 flags (and I could change my patch to have "p" be a mode of its own instead of modifying the "f" mode). But like I was saying, I'm open to changing it. I figured this would probably be the main debate with this patch. > Unrelated changes? Woops, yes. Sorry, I didn't correctly check my cmdline arguments when sending the patch. -- l u d o . . 8 0 17 80 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Using mercurial on windows.
> After sometime of fighting with windows, here I am looking for suggestions on
> how one can use mercurial on windows smoothly and what are the recommended
> ways. Like shall I use MYSYS, did things will work in cmd.exe?
>
> If anyone can shed some light on their experiences and tips and tricks on how
> to use mercurial on windows, that will be great!
Do you mean "using Mercurial on Windows" as a user, or did you really mean
"working with the Mercurial code on Windows"?
As a plain user, Mercurial is a breeze to use cross-platform, there's not much
to do. I personally use it in a normal cmd.exe shell (albeit inside a ConsoleZ
UI so I get resizable/tabbed terminals). Nothing fancy.
Checking my dotfiles (https://bitbucket.org/ludovicchabant/dotfiles), the only
tricks I have are:
1. Include an OS-specific config file from your .hgrc with:
%include hgrc-${OS}
(where $OS is a valid environment variable... I don't remember if I set it
myself or if it's set by default by most OSes, but for me on Windows 10 this
means it includes a config file named "hgrc-Windows_NT" in the same directory
as the main hgrc file)
2. In that OS-specific file, set the ui.ssh config to "plink.exe -ssh -2 -batch
-C" to make SSH pull/push work.
3. Set Windows-specific editor/extdiff/merge-tools configs if necessary.
That's it.
--
l u d o .
. 8 0 17 80
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] extdiff: add --mode option
# HG changeset patch
# User Ludovic Chabant
# Date 1546539395 28800
# Thu Jan 03 10:16:35 2019 -0800
# Node ID cc964d5d3e3faca1bbbc680732c38cf785754fc2
# Parent 5a2a6ab7bc37af9294b58edfd4d224706940ff19
extdiff: add --mode option
The new option lets the user control how the external program is run.
By default, Mercurial passes the 2 snapshot directories as usual, but
it can also run the program repeatedly on each file's snapshot pair,
and optionally prompt the user each time.
The option can be passed via the CLI or be set in the 'extdiff' tool
config.
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -71,6 +71,7 @@
import re
import shutil
import stat
+import subprocess
from mercurial.i18n import _
from mercurial.node import (
@@ -80,6 +81,7 @@
from mercurial import (
archival,
cmdutil,
+encoding,
error,
filemerge,
formatter,
@@ -104,6 +106,11 @@
generic=True,
)
+configitem('extdiff', br'mode\..*',
+default='',
+generic=True,
+)
+
configitem('diff-tools', br'.*\.diffargs$',
default=None,
generic=True,
@@ -152,6 +159,41 @@
fnsandstat.append((dest, repo.wjoin(fn), os.lstat(dest)))
return dirname, fnsandstat
+def formatcmdline(cmdline, repo_root, do3way,
+ parent1, plabel1, parent2, plabel2, child, clabel):
+# Function to quote file/dir names in the argument string.
+# When not operating in 3-way mode, an empty string is
+# returned for parent2
+replace = {'parent': parent1, 'parent1': parent1, 'parent2': parent2,
+ 'plabel1': plabel1, 'plabel2': plabel2,
+ 'child': child, 'clabel': clabel,
+ 'root': repo_root}
+def quote(match):
+pre = match.group(2)
+key = match.group(3)
+if not do3way and key == 'parent2':
+return pre
+return pre + procutil.shellquote(replace[key])
+
+# Match parent2 first, so 'parent1?' will match both parent1 and parent
+regex = (br'''(['"]?)([^\s'"$]*)'''
+ br'\$(parent2|parent1?|child|plabel1|plabel2|clabel|root)\1')
+if not do3way and not re.search(regex, cmdline):
+cmdline += ' $parent1 $child'
+return re.sub(regex, quote, cmdline)
+
+def _systemdetached(cmd, environ=None, cwd=None):
+''' like 'procutil.system', but returns the Popen object directly
+so we don't have to wait on it.
+'''
+cmd = procutil.quotecommand(cmd)
+env = procutil.shellenviron(environ)
+proc = subprocess.Popen(procutil.tonativestr(cmd),
+shell=True, close_fds=procutil.closefds,
+env=procutil.tonativeenv(env),
+cwd=pycompat.rapply(procutil.tonativestr, cwd))
+return proc
+
def dodiff(ui, repo, cmdline, pats, opts):
'''Do the actual diff:
@@ -178,6 +220,8 @@
else:
ctx1b = repo[nullid]
+diffmode = opts.get('mode')
+
node1a = ctx1a.node()
node1b = ctx1b.node()
node2 = ctx2.node()
@@ -194,6 +238,8 @@
if opts.get('patch'):
if subrepos:
raise error.Abort(_('--patch cannot be used with --subrepos'))
+if diffmode:
+raise error.Abort(_('--patch cannot be used with --mode'))
if node2 is None:
raise error.Abort(_('--patch requires two revisions'))
else:
@@ -281,29 +327,86 @@
label1b = None
fnsandstat = []
-# Function to quote file/dir names in the argument string.
-# When not operating in 3-way mode, an empty string is
-# returned for parent2
-replace = {'parent': dir1a, 'parent1': dir1a, 'parent2': dir1b,
- 'plabel1': label1a, 'plabel2': label1b,
- 'clabel': label2, 'child': dir2,
- 'root': repo.root}
-def quote(match):
-pre = match.group(2)
-key = match.group(3)
-if not do3way and key == 'parent2':
-return pre
-return pre + procutil.shellquote(replace[key])
+if not diffmode or 'd' in diffmode:
+# Run the external tool on the 2 temp directories or the patches
+cmdline = formatcmdline(
+cmdline, repo.root, do3way=do3way,
+parent1=dir1a, plabel1=label1a,
+parent2=dir1b, plabel2=label1b,
+child=dir2, clabel=label2)
+ui.debug('running %r in %s\n' % (pyco
[PATCH] help: fix typo
# HG changeset patch # User Ludovic Chabant # Date 1546539214 28800 # Thu Jan 03 10:13:34 2019 -0800 # Node ID 5a2a6ab7bc37af9294b58edfd4d224706940ff19 # Parent 5c68b617ba2463eb6f1372a24b139a376c6bf6bd help: fix typo diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt --- a/mercurial/help/config.txt +++ b/mercurial/help/config.txt @@ -462,7 +462,7 @@ (default: False) ``status.terse`` -Default value for the --terse flag, which condenes status output. +Default value for the --terse flag, which condenses status output. (default: empty) ``update.check`` ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
