editor: use an unambiguous path suffix for editor files
authorMichael Bolin <mbolin@fb.com>
Wed, 30 Aug 2017 20:25:56 +0000
changeset 34029 6e6452bc441d
parent 34028 bfb38c5cebf4
child 34030 e97be042fa1b
editor: use an unambiguous path suffix for editor files Changes the API of `ui.edit()` to take an optional `action` argument, which is used when constructing the suffix of the temp file. Previously, it was possible to set the suffix by specifying a `suffix` to the optional `extra` dict that was passed to `ui.edit()`, but the goal is to drop support for `extra.suffix` and make `action` a required argument. To this end, `ui.edit()` now yields a `develwarn()` if `action` is not set or if `extra.suffix` is set. I updated all calls to `ui.edit()` I could find in `hg-crew` to specify the appropriate `action`. This means that when creating a commit, instead of the path to the editor file being something like: `/tmp/hg-editor-XXXXXX.txt` it is now something like: `/tmp/hg-editor-XXXXXX.commit.hg.txt` Some editors (such as Atom) make it possible to statically define a [TextMate] grammar for files with a particular suffix. For example, because Git reliably uses `.git/COMMIT_EDITMSG` and `.git/MERGE_MSG` as the paths for commit-type messages, it is trivial to define a grammar that is applied when files of either name are opened in Atom: https://github.com/atom/language-git/blob/v0.19.1/grammars/git%20commit%20message.cson#L4-L5 Because Hg historically used a generic `.txt` suffix, it was much harder to disambiguate whether a file was an arbitrary text file as opposed to one created for the specific purpose of authoring an Hg commit message. This also makes it easier to add special support for `histedit`, as it has its own suffix that is distinct from a commit: `/tmp/hg-histedit-XXXXXX.histedit.hg.txt` Test Plan: Added an integration test: `test-editor-filename.t`. Manually tested: ran `hg ci --amend` for this change and saw that it used `/tmp/hg-editor-ZZjcz0.commit.hg.txt` as the path instead of `/tmp/hg-editor-ZZjcz0.txt` as the path. Verified `make tests` passes. Differential Revision: https://phab.mercurial-scm.org/D464
hgext/histedit.py
hgext/patchbomb.py
mercurial/cmdutil.py
mercurial/crecord.py
mercurial/ui.py
tests/test-editor-filename.t
--- a/hgext/histedit.py	Wed Aug 30 09:21:31 2017 -0700
+++ b/hgext/histedit.py	Wed Aug 30 20:25:56 2017 +0000
@@ -1370,7 +1370,7 @@
     rules += '\n\n'
     rules += editcomment
     rules = ui.edit(rules, ui.username(), {'prefix': 'histedit'},
-                    repopath=repo.path)
+                    repopath=repo.path, action='histedit')
 
     # Save edit rules in .hg/histedit-last-edit.txt in case
     # the user needs to ask for help after something
--- a/hgext/patchbomb.py	Wed Aug 30 09:21:31 2017 -0700
+++ b/hgext/patchbomb.py	Wed Aug 30 20:25:56 2017 +0000
@@ -308,7 +308,8 @@
     else:
         ui.write(_('\nWrite the introductory message for the '
                    'patch series.\n\n'))
-        body = ui.edit(defaultbody, sender, repopath=repo.path)
+        body = ui.edit(defaultbody, sender, repopath=repo.path,
+                       action='patchbombbody')
         # Save series description in case sendmail fails
         msgfile = repo.vfs('last-email.txt', 'wb')
         msgfile.write(body)
--- a/mercurial/cmdutil.py	Wed Aug 30 09:21:31 2017 -0700
+++ b/mercurial/cmdutil.py	Wed Aug 30 20:25:56 2017 +0000
@@ -341,7 +341,7 @@
                              + crecordmod.patchhelptext
                              + fp.read())
                 reviewedpatch = ui.edit(patchtext, "",
-                                        extra={"suffix": ".diff"},
+                                        action="diff",
                                         repopath=repo.path)
                 fp.truncate(0)
                 fp.write(reviewedpatch)
@@ -3217,7 +3217,7 @@
 
     editortext = repo.ui.edit(committext, ctx.user(), ctx.extra(),
                               editform=editform, pending=pending,
-                              repopath=repo.path)
+                              repopath=repo.path, action='commit')
     text = editortext
 
     # strip away anything below this special string (used for editors that want
--- a/mercurial/crecord.py	Wed Aug 30 09:21:31 2017 -0700
+++ b/mercurial/crecord.py	Wed Aug 30 20:25:56 2017 +0000
@@ -1563,8 +1563,7 @@
 
             # start the editor and wait for it to complete
             try:
-                patch = self.ui.edit(patch.getvalue(), "",
-                                     extra={"suffix": ".diff"})
+                patch = self.ui.edit(patch.getvalue(), "", action="diff")
             except error.Abort as exc:
                 self.errorstr = str(exc)
                 return None
--- a/mercurial/ui.py	Wed Aug 30 09:21:31 2017 -0700
+++ b/mercurial/ui.py	Wed Aug 30 20:25:56 2017 +0000
@@ -1346,20 +1346,31 @@
             self.write(*msg, **opts)
 
     def edit(self, text, user, extra=None, editform=None, pending=None,
-             repopath=None):
+             repopath=None, action=None):
+        if action is None:
+            self.develwarn('action is None but will soon be a required '
+                           'parameter to ui.edit()')
         extra_defaults = {
             'prefix': 'editor',
             'suffix': '.txt',
         }
         if extra is not None:
+            if extra.get('suffix') is not None:
+                self.develwarn('extra.suffix is not None but will soon be '
+                               'ignored by ui.edit()')
             extra_defaults.update(extra)
         extra = extra_defaults
 
+        if action:
+            suffix = '.%s.hg.txt' % action
+        else:
+            suffix = extra['suffix']
+
         rdir = None
         if self.configbool('experimental', 'editortmpinhg'):
             rdir = repopath
         (fd, name) = tempfile.mkstemp(prefix='hg-' + extra['prefix'] + '-',
-                                      suffix=extra['suffix'],
+                                      suffix=suffix,
                                       dir=rdir)
         try:
             f = os.fdopen(fd, r'wb')
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-editor-filename.t	Wed Aug 30 20:25:56 2017 +0000
@@ -0,0 +1,35 @@
+Test temp file used with an editor has the expected suffix.
+
+  $ hg init
+
+Create an editor that writes its arguments to stdout and set it to $HGEDITOR.
+
+  $ cat > editor.sh << EOF
+  > #!/bin/bash
+  > echo "\$@"
+  > exit 1
+  > EOF
+  $ chmod +x editor.sh
+  $ hg add editor.sh
+  $ HGEDITOR=$TESTTMP/editor.sh
+  $ export HGEDITOR
+
+Verify that the path for a commit editor has the expected suffix.
+
+  $ hg commit
+  *.commit.hg.txt (glob)
+  abort: edit failed: editor.sh exited with status 1
+  [255]
+
+Verify that the path for a histedit editor has the expected suffix.
+
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > rebase=
+  > histedit=
+  > EOF
+  $ hg commit --message 'At least one commit for histedit.'
+  $ hg histedit
+  *.histedit.hg.txt (glob)
+  abort: edit failed: editor.sh exited with status 1
+  [255]