D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG04e81ae47066: debugcommands: print out the editor that was
searched for (post shlexsplit) (authored by spectral, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D1808?vs=4714&id=5783
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
AFFECTED FILES
mercurial/debugcommands.py
tests/test-install.t
CHANGE DETAILS
diff --git a/tests/test-install.t b/tests/test-install.t
--- a/tests/test-install.t
+++ b/tests/test-install.t
@@ -17,7 +17,7 @@
checking "re2" regexp engine \((available|missing)\) (re)
checking templates (*mercurial?templates)... (glob)
checking default template (*mercurial?templates?map-cmdline.default) (glob)
- checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
+ checking commit editor... (*) (glob)
checking username (test)
no problems detected
@@ -31,7 +31,7 @@
"defaulttemplate": "*mercurial?templates?map-cmdline.default", (glob)
"defaulttemplateerror": null,
"defaulttemplatenotfound": "default",
-"editor": "* -c \"import sys; sys.exit(0)\"", (glob)
+"editor": "*", (glob)
"editornotfound": false,
"encoding": "ascii",
"encodingerror": null,
@@ -72,7 +72,7 @@
checking "re2" regexp engine \((available|missing)\) (re)
checking templates (*mercurial?templates)... (glob)
checking default template (*mercurial?templates?map-cmdline.default) (glob)
- checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
+ checking commit editor... (*) (glob)
checking username...
no username supplied
(specify a username in your configuration file)
@@ -120,6 +120,33 @@
checking username (test)
no problems detected
+print out the binary post-shlexsplit in the error message when commit editor is
+not found (this is intentionally using backslashes to mimic a windows usecase).
+ $ HGEDITOR="c:\foo\bar\baz.exe -y -z" hg debuginstall
+ checking encoding (ascii)...
+ checking Python executable (*) (glob)
+ checking Python version (*) (glob)
+ checking Python lib (*lib*)... (glob)
+ checking Python security support (*) (glob)
+TLS 1.2 not supported by Python install; network connections lack modern
security (?)
+SNI not supported by Python install; may have connectivity issues with
some servers (?)
+ checking Mercurial version (*) (glob)
+ checking Mercurial custom build (*) (glob)
+ checking module policy (*) (glob)
+ checking installed modules (*mercurial)... (glob)
+ checking registered compression engines (*zlib*) (glob)
+ checking available compression engines (*zlib*) (glob)
+ checking available compression engines for wire protocol (*zlib*) (glob)
+ checking "re2" regexp engine \((available|missing)\) (re)
+ checking templates (*mercurial?templates)... (glob)
+ checking default template (*mercurial?templates?map-cmdline.default) (glob)
+ checking commit editor... (c:foobarbaz.exe)
+ Can't find editor 'c:foobarbaz.exe' in PATH
+ (specify a commit editor in your configuration file)
+ checking username (test)
+ 1 problems detected, please check your install!
+ [1]
+
#if test-repo
$ . "$TESTDIR/helpers-testrepo.sh"
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -1240,16 +1240,17 @@
# editor
editor = ui.geteditor()
editor = util.expandpath(editor)
-fm.write('editor', _("checking commit editor... (%s)\n"), editor)
-cmdpath = util.findexe(pycompat.shlexsplit(editor)[0])
+editorbin = pycompat.shlexsplit(editor)[0]
+fm.write('editor', _("checking commit editor... (%s)\n"), editorbin)
+cmdpath = util.findexe(editorbin)
fm.condwrite(not cmdpath and editor == 'vi', 'vinotfound',
_(" No commit editor set and can't find %s in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor == 'vi' and editor)
+ " file)\n"), not cmdpath and editor == 'vi' and editorbin)
fm.condwrite(not cmdpath and editor != 'vi', 'editornotfound',
_(" Can't find editor '%s' in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor)
+ " file)\n"), not cmdpath and editorbin)
if not cmdpath and editor != 'vi':
problems += 1
To: spectral, #hg-reviewers, krbullock
Cc: durin42, krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
krbullock added a comment. @durin42 Go for it REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1808 To: spectral, #hg-reviewers, krbullock Cc: durin42, krbullock, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
durin42 added a comment. This looks good to me - @krbullock should I queue it? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1808 To: spectral, #hg-reviewers, krbullock Cc: durin42, krbullock, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
krbullock accepted this revision as: krbullock. krbullock added a comment. This LGTM now. I think the executable as interpreted by shlex, but without the added flags, gives enough signal for the user to figure out what might be wrong without confusing things. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1808 To: spectral, #hg-reviewers, krbullock Cc: krbullock, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
spectral marked an inline comment as done. spectral added a comment. Sorry, I forgot about this, can you take another look? I believe I addressed your most recent comment. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1808 To: spectral, #hg-reviewers, krbullock Cc: krbullock, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
spectral added inline comments.
INLINE COMMENTS
> krbullock wrote in debugcommands.py:1212
> In this patch, we end up printing something potentially different in the
> "checking" line than in the "can't find editor" line. That seems wrong. I
> think I'd rather we do the shlexsplit before writing the "checking commit
> editor..." message and print the fully-interpreted path there.
I'm not sure I agree, but I'm not sure I disagree either :)
Anything using this output (it shouldn't afaik, but the user report in
#mercurial said that this broke Eclipse, maybe it was only looking at
debuginstall to see if there were problems and not actually parsing these
values...?) might end up with incorrect results.
- If we limit both of them to [0], it will end up with just the executable and
miss the flags. Maybe this is fine? I don't know what Eclipse is doing with
this command, basically, but I don't think we have BC guarantees on debug*
commands.
- If either or both of them are not limited to just [0], then they have to be
output as something like `"c:\foo\bar.exe" "-arg1" "-arg2"` to show where the
actual word boundaries are; we can't do something like:
`' '.join(shlexed_editor)`
We have to output it like that to avoid a similar problem, where instead of:
[ui]
editor = c:\foo\bar.exe -arg1 -arg2
the user wrote:
[ui]
editor = "c:\foo\bar.exe -arg1 -arg2"
(Currently, that problem is slightly more obvious, since the output is
`checking commit editor... ("c:\foo\bar.exe -arg1 -arg2")`).
Basically... if we're ok with the rather mild BC break, I think it's probably
fine to list only the editor, post-shlexsplit. I don't know what the
guarantees are here. Anticipating that we're going to be OK with the output
difference, since I can't see how things can actually correctly rely on the
non-shlexsplit, non-findexe'd value we emit here (and thus have to assume that
either nothing does, or they're already broken), I've made the change.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
To: spectral, #hg-reviewers, krbullock
Cc: krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
spectral updated this revision to Diff 4714.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D1808?vs=4695&id=4714
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
AFFECTED FILES
mercurial/debugcommands.py
tests/test-install.t
CHANGE DETAILS
diff --git a/tests/test-install.t b/tests/test-install.t
--- a/tests/test-install.t
+++ b/tests/test-install.t
@@ -17,7 +17,7 @@
checking "re2" regexp engine \((available|missing)\) (re)
checking templates (*mercurial?templates)... (glob)
checking default template (*mercurial?templates?map-cmdline.default) (glob)
- checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
+ checking commit editor... (*) (glob)
checking username (test)
no problems detected
@@ -31,7 +31,7 @@
"defaulttemplate": "*mercurial?templates?map-cmdline.default", (glob)
"defaulttemplateerror": null,
"defaulttemplatenotfound": "default",
-"editor": "* -c \"import sys; sys.exit(0)\"", (glob)
+"editor": "*", (glob)
"editornotfound": false,
"encoding": "ascii",
"encodingerror": null,
@@ -72,7 +72,7 @@
checking "re2" regexp engine \((available|missing)\) (re)
checking templates (*mercurial?templates)... (glob)
checking default template (*mercurial?templates?map-cmdline.default) (glob)
- checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
+ checking commit editor... (*) (glob)
checking username...
no username supplied
(specify a username in your configuration file)
@@ -120,6 +120,33 @@
checking username (test)
no problems detected
+print out the binary post-shlexsplit in the error message when commit editor is
+not found (this is intentionally using backslashes to mimic a windows usecase).
+ $ HGEDITOR="c:\foo\bar\baz.exe -y -z" hg debuginstall
+ checking encoding (ascii)...
+ checking Python executable (*) (glob)
+ checking Python version (*) (glob)
+ checking Python lib (*lib*)... (glob)
+ checking Python security support (*) (glob)
+TLS 1.2 not supported by Python install; network connections lack modern
security (?)
+SNI not supported by Python install; may have connectivity issues with
some servers (?)
+ checking Mercurial version (*) (glob)
+ checking Mercurial custom build (*) (glob)
+ checking module policy (*) (glob)
+ checking installed modules (*mercurial)... (glob)
+ checking registered compression engines (*zlib*) (glob)
+ checking available compression engines (*zlib*) (glob)
+ checking available compression engines for wire protocol (*zlib*) (glob)
+ checking "re2" regexp engine \((available|missing)\) (re)
+ checking templates (*mercurial?templates)... (glob)
+ checking default template (*mercurial?templates?map-cmdline.default) (glob)
+ checking commit editor... (c:foobarbaz.exe)
+ Can't find editor 'c:foobarbaz.exe' in PATH
+ (specify a commit editor in your configuration file)
+ checking username (test)
+ 1 problems detected, please check your install!
+ [1]
+
#if test-repo
$ . "$TESTDIR/helpers-testrepo.sh"
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -1208,16 +1208,17 @@
# editor
editor = ui.geteditor()
editor = util.expandpath(editor)
-fm.write('editor', _("checking commit editor... (%s)\n"), editor)
-cmdpath = util.findexe(pycompat.shlexsplit(editor)[0])
+editorbin = pycompat.shlexsplit(editor)[0]
+fm.write('editor', _("checking commit editor... (%s)\n"), editorbin)
+cmdpath = util.findexe(editorbin)
fm.condwrite(not cmdpath and editor == 'vi', 'vinotfound',
_(" No commit editor set and can't find %s in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor == 'vi' and editor)
+ " file)\n"), not cmdpath and editor == 'vi' and editorbin)
fm.condwrite(not cmdpath and editor != 'vi', 'editornotfound',
_(" Can't find editor '%s' in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor)
+ " file)\n"), not cmdpath and editorbin)
if not cmdpath and editor != 'vi':
problems += 1
To: spectral, #hg-reviewers, krbullock
Cc: krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
krbullock requested changes to this revision.
krbullock added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> debugcommands.py:1212
> fm.write('editor', _("checking commit editor... (%s)\n"), editor)
> -cmdpath = util.findexe(pycompat.shlexsplit(editor)[0])
> +editorbin = pycompat.shlexsplit(editor)[0]
> +cmdpath = util.findexe(editorbin)
In this patch, we end up printing something potentially different in the
"checking" line than in the "can't find editor" line. That seems wrong. I think
I'd rather we do the shlexsplit before writing the "checking commit editor..."
message and print the fully-interpreted path there.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
To: spectral, #hg-reviewers, krbullock
Cc: krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
spectral updated this revision to Diff 4695.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D1808?vs=4694&id=4695
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
AFFECTED FILES
mercurial/debugcommands.py
tests/test-install.t
CHANGE DETAILS
diff --git a/tests/test-install.t b/tests/test-install.t
--- a/tests/test-install.t
+++ b/tests/test-install.t
@@ -120,6 +120,33 @@
checking username (test)
no problems detected
+print out the binary post-shlexsplit in the error message when commit editor is
+not found (this is intentionally using backslashes to mimic a windows usecase).
+ $ HGEDITOR="c:\foo\bar\baz.exe -y -z" hg debuginstall
+ checking encoding (ascii)...
+ checking Python executable (*) (glob)
+ checking Python version (*) (glob)
+ checking Python lib (*lib*)... (glob)
+ checking Python security support (*) (glob)
+TLS 1.2 not supported by Python install; network connections lack modern
security (?)
+SNI not supported by Python install; may have connectivity issues with
some servers (?)
+ checking Mercurial version (*) (glob)
+ checking Mercurial custom build (*) (glob)
+ checking module policy (*) (glob)
+ checking installed modules (*mercurial)... (glob)
+ checking registered compression engines (*zlib*) (glob)
+ checking available compression engines (*zlib*) (glob)
+ checking available compression engines for wire protocol (*zlib*) (glob)
+ checking "re2" regexp engine \((available|missing)\) (re)
+ checking templates (*mercurial?templates)... (glob)
+ checking default template (*mercurial?templates?map-cmdline.default) (glob)
+ checking commit editor... (c:\foo\bar\baz.exe -y -z)
+ Can't find editor 'c:foobarbaz.exe' in PATH
+ (specify a commit editor in your configuration file)
+ checking username (test)
+ 1 problems detected, please check your install!
+ [1]
+
#if test-repo
$ . "$TESTDIR/helpers-testrepo.sh"
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -1209,15 +1209,16 @@
editor = ui.geteditor()
editor = util.expandpath(editor)
fm.write('editor', _("checking commit editor... (%s)\n"), editor)
-cmdpath = util.findexe(pycompat.shlexsplit(editor)[0])
+editorbin = pycompat.shlexsplit(editor)[0]
+cmdpath = util.findexe(editorbin)
fm.condwrite(not cmdpath and editor == 'vi', 'vinotfound',
_(" No commit editor set and can't find %s in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor == 'vi' and editor)
+ " file)\n"), not cmdpath and editor == 'vi' and editorbin)
fm.condwrite(not cmdpath and editor != 'vi', 'editornotfound',
_(" Can't find editor '%s' in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor)
+ " file)\n"), not cmdpath and editorbin)
if not cmdpath and editor != 'vi':
problems += 1
To: spectral, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
spectral updated this revision to Diff 4694.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D1808?vs=4692&id=4694
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
AFFECTED FILES
mercurial/debugcommands.py
tests/test-install.t
CHANGE DETAILS
diff --git a/tests/test-install.t b/tests/test-install.t
--- a/tests/test-install.t
+++ b/tests/test-install.t
@@ -120,6 +120,34 @@
checking username (test)
no problems detected
+print out the binary post-shlexsplit in the error message when commit editor is
+not found (this is intentionally using backslashes to mimic a windows usecase).
+ $ HGEDITOR="c:\foo\bar\baz.exe -y -z" hg debuginstall
+ checking encoding (ascii)...
+ checking Python executable (*) (glob)
+ checking Python version (*) (glob)
+ checking Python lib (*lib*)... (glob)
+ checking Python security support (*) (glob)
+TLS 1.2 not supported by Python install; network connections lack modern
security (?)
+SNI not supported by Python install; may have connectivity issues with
some servers (?)
+ checking Mercurial version (*) (glob)
+ checking Mercurial custom build (*) (glob)
+ checking module policy (*) (glob)
+ checking installed modules (*mercurial)... (glob)
+ checking registered compression engines (*zlib*) (glob)
+ checking available compression engines (*zlib*) (glob)
+ checking available compression engines for wire protocol (*zlib*) (glob)
+ checking "re2" regexp engine \((available|missing)\) (re)
+ checking templates (*mercurial?templates)... (glob)
+ checking default template (*mercurial?templates?map-cmdline.default) (glob)
+ checking commit editor... (c:\foo\bar\baz.exe -y -z)
+ Can't find editor 'c:foobarbaz.exe' in PATH
+ (specify a commit editor in your configuration file)
+ checking username (test)
+ 1 problems detected, please check your install!
+ [1]
+
+
#if test-repo
$ . "$TESTDIR/helpers-testrepo.sh"
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -1209,15 +1209,16 @@
editor = ui.geteditor()
editor = util.expandpath(editor)
fm.write('editor', _("checking commit editor... (%s)\n"), editor)
-cmdpath = util.findexe(pycompat.shlexsplit(editor)[0])
+editorbin = pycompat.shlexsplit(editor)[0]
+cmdpath = util.findexe(editorbin)
fm.condwrite(not cmdpath and editor == 'vi', 'vinotfound',
_(" No commit editor set and can't find %s in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor == 'vi' and editor)
+ " file)\n"), not cmdpath and editor == 'vi' and editorbin)
fm.condwrite(not cmdpath and editor != 'vi', 'editornotfound',
_(" Can't find editor '%s' in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor)
+ " file)\n"), not cmdpath and editorbin)
if not cmdpath and editor != 'vi':
problems += 1
To: spectral, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1808: debugcommands: print out the editor that was searched for (post shlexsplit)
spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
A user was in #mercurial and had the following settings in their hgrc:
[ui]
editor = C:\home\npp\notepad++.exe -multiInst -nosession
After shlexsplit, the first argument was mangled into C:homenppnotepad++.exe,
which was quite unlikely to exist. It took many back-and-forths to identify
that adding " characters around the exe would fix the issue; we were thinking
that it's because something was incorrectly *not* splitting and adding/moving
the " characters fixed the split boundaries, but when testing afterward it
appears that it's just mangled.
I considered adding an informational if pycompat.iswindows and \ in the string
about this issue, but was worried that might have too many false positives and
did not do so at this time.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1808
AFFECTED FILES
mercurial/debugcommands.py
CHANGE DETAILS
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -1209,15 +1209,16 @@
editor = ui.geteditor()
editor = util.expandpath(editor)
fm.write('editor', _("checking commit editor... (%s)\n"), editor)
-cmdpath = util.findexe(pycompat.shlexsplit(editor)[0])
+editorbin = pycompat.shlexsplit(editor)[0]
+cmdpath = util.findexe(editorbin)
fm.condwrite(not cmdpath and editor == 'vi', 'vinotfound',
_(" No commit editor set and can't find %s in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor == 'vi' and editor)
+ " file)\n"), editorbin)
fm.condwrite(not cmdpath and editor != 'vi', 'editornotfound',
_(" Can't find editor '%s' in PATH\n"
" (specify a commit editor in your configuration"
- " file)\n"), not cmdpath and editor)
+ " file)\n"), editorbin)
if not cmdpath and editor != 'vi':
problems += 1
To: spectral, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
