Patchwork [2,of,2] rebase: ignore commands.rebase.requiredest if HGPLAIN=1

login
register
mail settings
Submitter via Mercurial-devel
Date March 22, 2017, 4:27 a.m.
Message ID <23f767fb3142615597c4.1490156841@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/19548/
State Changes Requested
Headers show

Comments

via Mercurial-devel - March 22, 2017, 4:27 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1490156812 25200
#      Tue Mar 21 21:26:52 2017 -0700
# Node ID 23f767fb3142615597c4d3c2d5b4404e6c10f57a
# Parent  2558f3d814f50681641fff9815d30129de2ab5ad
rebase: ignore commands.rebase.requiredest if HGPLAIN=1
Ryan McElroy - March 22, 2017, 10:43 a.m.
On 3/22/17 4:27 AM, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1490156812 25200
> #      Tue Mar 21 21:26:52 2017 -0700
> # Node ID 23f767fb3142615597c4d3c2d5b4404e6c10f57a
> # Parent  2558f3d814f50681641fff9815d30129de2ab5ad
> rebase: ignore commands.rebase.requiredest if HGPLAIN=1

Thanks for the follow-up! Both patches in this series look correct to me 
and fit the hg philosophy. They should be queued.

===

Disclaimer: All of the following is not an objection to this series. 
This series should be queued as-is. I want to start a broader discussion 
here that ties into the "compatibility levels" discussion we had at the 
sprint, but is a little more specific to the implementation of HGPLAIN.

Zooming out a level, if HGPLAIN remains a large hammer that disables all 
ui modernizations, I wonder if we should be thinking about a way to get 
output intended for tools but still have some modernizations (for 
example, relative paths in your recent patch). Otherwise, we'll 
eventually have two totally different hg's: the HGPLAIN one which 
behaves like a 2005-era VCS, and the one humans use which behaves like a 
2020-era VCS. Eventually That will make fewer people want to use HGPLAIN 
for their scripts, which is a bad outcome.

We need some way of saying "I'm a script but I still want hg to behave 
in a modern way". I guess the compatibility layer stuff can achieve this 
-- perhaps by making some options ignore HGPLAIN?

After having thought it through a bit while writing this, I think this 
is the way to go (please poke holes it it if you find any of course) -- 
the upcoming config registry we agreed to at the sprint should probably 
handle everything to do with HGPLAIN as well. Then we don't need HGPLAIN 
checks scattered throughout the code -- HGPLAIN consideration is taken 
into account at the config registry level. Not only will this make 
things more consistent and less buggy (like, I wouldn't be able to 
forget to add HGPLAIN support that you're fixing up here), but also it 
will provide a path for certain compatibility levels to ignore HGPLAIN 
for some settings.

Thoughts?


>
> diff -r 2558f3d814f5 -r 23f767fb3142 hgext/rebase.py
> --- a/hgext/rebase.py	Tue Mar 21 21:22:00 2017 -0700
> +++ b/hgext/rebase.py	Tue Mar 21 21:26:52 2017 -0700
> @@ -686,7 +686,8 @@
>           # Validate input and define rebasing points
>           destf = opts.get('dest', None)
>   
> -        if ui.config('commands', 'rebase.requiredest', False):
> +        if (not ui.plain() and
> +            ui.config('commands', 'rebase.requiredest', False)):
>               if not destf:
>                   raise error.Abort(_('you must specify a destination'),
>                                     hint=_('use: hg rebase -d REV'))
> diff -r 2558f3d814f5 -r 23f767fb3142 tests/test-rebase-base.t
> --- a/tests/test-rebase-base.t	Tue Mar 21 21:22:00 2017 -0700
> +++ b/tests/test-rebase-base.t	Tue Mar 21 21:26:52 2017 -0700
> @@ -413,3 +413,7 @@
>     $ hg rebase -d 1
>     rebasing 2:5db65b93a12b "cc" (tip)
>     saved backup bundle to $TESTTMP/repo/.hg/strip-backup/5db65b93a12b-4fb789ec-backup.hg (glob)
> +  $ hg rebase -d 0 -r . -q
> +  $ HGPLAIN=1 hg rebase
> +  rebasing 2:889b0bc6a730 "cc" (tip)
> +  saved backup bundle to $TESTTMP/repo/.hg/strip-backup/889b0bc6a730-41ec4f81-backup.hg (glob)
>

Patch

diff -r 2558f3d814f5 -r 23f767fb3142 hgext/rebase.py
--- a/hgext/rebase.py	Tue Mar 21 21:22:00 2017 -0700
+++ b/hgext/rebase.py	Tue Mar 21 21:26:52 2017 -0700
@@ -686,7 +686,8 @@ 
         # Validate input and define rebasing points
         destf = opts.get('dest', None)
 
-        if ui.config('commands', 'rebase.requiredest', False):
+        if (not ui.plain() and
+            ui.config('commands', 'rebase.requiredest', False)):
             if not destf:
                 raise error.Abort(_('you must specify a destination'),
                                   hint=_('use: hg rebase -d REV'))
diff -r 2558f3d814f5 -r 23f767fb3142 tests/test-rebase-base.t
--- a/tests/test-rebase-base.t	Tue Mar 21 21:22:00 2017 -0700
+++ b/tests/test-rebase-base.t	Tue Mar 21 21:26:52 2017 -0700
@@ -413,3 +413,7 @@ 
   $ hg rebase -d 1
   rebasing 2:5db65b93a12b "cc" (tip)
   saved backup bundle to $TESTTMP/repo/.hg/strip-backup/5db65b93a12b-4fb789ec-backup.hg (glob)
+  $ hg rebase -d 0 -r . -q
+  $ HGPLAIN=1 hg rebase
+  rebasing 2:889b0bc6a730 "cc" (tip)
+  saved backup bundle to $TESTTMP/repo/.hg/strip-backup/889b0bc6a730-41ec4f81-backup.hg (glob)