Patchwork [6,of,6,RFC] push: allow specifying default-push as defaultpush

login
register
mail settings
Submitter Augie Fackler
Date Aug. 2, 2013, 2:15 p.m.
Message ID <d59a05f3c4e013ecf9c7.1375452905@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/1987/
State Deferred
Headers show

Comments

Augie Fackler - Aug. 2, 2013, 2:15 p.m.
# HG changeset patch
# User Augie Fackler <durin42@gmail.com>
# Date 1374709814 14400
#      Wed Jul 24 19:50:14 2013 -0400
# Node ID d59a05f3c4e013ecf9c7d3b40c04fa9900b7f74f
# Parent  4c9c2538d46bed89734a465ae01dbda483154425
push: allow specifying default-push as defaultpush
Martin Geisler - Aug. 7, 2013, 4:06 p.m.
Kevin Bullock <kbullock+mercurial@ringworld.org> writes:

> On 2 Aug 2013, at 9:15 AM, Augie Fackler wrote:
>
>> # HG changeset patch
>> # User Augie Fackler <durin42@gmail.com>
>> # Date 1374709814 14400
>> #      Wed Jul 24 19:50:14 2013 -0400
>> # Node ID d59a05f3c4e013ecf9c7d3b40c04fa9900b7f74f
>> # Parent  4c9c2538d46bed89734a465ae01dbda483154425
>> push: allow specifying default-push as defaultpush
>
> Unlike the rest of the series, this one causes us to change the output
> -- on one of our most well-publicized config options. I guess I'm -1
> on this until we're ready to officially favor the
> non-hyphenated/-underscored forms. Rest of the series looks fine to
> me.

Why would you even prefer the non-hyphenated versions? I know we use
names without underscores in the Python code, but I think someone needs
to explain how the arguments for that carry over to the config keys.

I think that having no underscores or hyphens comes at a cost in
readability and our docs might look more friendly if people see

  [paths]
  default-push = http://...

instead of the version without a hyphen. I realize, though, that we have
a lot more options with run-together words.
Augie Fackler - Aug. 7, 2013, 5:31 p.m.
On Wed, Aug 07, 2013 at 06:06:53PM +0200, Martin Geisler wrote:
> Kevin Bullock <kbullock+mercurial@ringworld.org> writes:
>
> > On 2 Aug 2013, at 9:15 AM, Augie Fackler wrote:
> >
> >> # HG changeset patch
> >> # User Augie Fackler <durin42@gmail.com>
> >> # Date 1374709814 14400
> >> #      Wed Jul 24 19:50:14 2013 -0400
> >> # Node ID d59a05f3c4e013ecf9c7d3b40c04fa9900b7f74f
> >> # Parent  4c9c2538d46bed89734a465ae01dbda483154425
> >> push: allow specifying default-push as defaultpush
> >
> > Unlike the rest of the series, this one causes us to change the output
> > -- on one of our most well-publicized config options. I guess I'm -1
> > on this until we're ready to officially favor the
> > non-hyphenated/-underscored forms. Rest of the series looks fine to
> > me.
>
> Why would you even prefer the non-hyphenated versions? I know we use
> names without underscores in the Python code, but I think someone needs
> to explain how the arguments for that carry over to the config keys.
>
> I think that having no underscores or hyphens comes at a cost in
> readability and our docs might look more friendly if people see
>
>   [paths]
>   default-push = http://...
>
> instead of the version without a hyphen. I realize, though, that we have
> a lot more options with run-together words.

As we add configuration options, they will be without dashes or
underscores, and it'd be nice to slowly converge on a single format
instead of the patchwork quilt we have now.

>
> --
> Martin Geisler
Matt Mackall - Aug. 7, 2013, 9:22 p.m.
On Wed, 2013-08-07 at 13:31 -0400, Augie Fackler wrote:
> On Wed, Aug 07, 2013 at 06:06:53PM +0200, Martin Geisler wrote:
> > Kevin Bullock <kbullock+mercurial@ringworld.org> writes:
> >
> > > On 2 Aug 2013, at 9:15 AM, Augie Fackler wrote:
> > >
> > >> # HG changeset patch
> > >> # User Augie Fackler <durin42@gmail.com>
> > >> # Date 1374709814 14400
> > >> #      Wed Jul 24 19:50:14 2013 -0400
> > >> # Node ID d59a05f3c4e013ecf9c7d3b40c04fa9900b7f74f
> > >> # Parent  4c9c2538d46bed89734a465ae01dbda483154425
> > >> push: allow specifying default-push as defaultpush
> > >
> > > Unlike the rest of the series, this one causes us to change the output
> > > -- on one of our most well-publicized config options. I guess I'm -1
> > > on this until we're ready to officially favor the
> > > non-hyphenated/-underscored forms. Rest of the series looks fine to
> > > me.
> >
> > Why would you even prefer the non-hyphenated versions? I know we use
> > names without underscores in the Python code, but I think someone needs
> > to explain how the arguments for that carry over to the config keys.

The argument has never been "underscores are bad", it's always been
"consistency is good". And that argument obviously applies to config
keys just as well as source code identifiers.

"How do I fix this?"

"You just need to set default push."

"Is that default no space push, default underscore push, or default dash
push?"

"Default dash push. Then set allow push and allow pull on the server."

"They don't do anything?!"

"Oh, sorry, that's allow underscore push and allow no space pull."

"Are you freaking kidding me?"


I'm not particularly attached to a particular choice of style, just that
_there is a style_ and people (like me, fercrissake) don't need to
repeatedly consult the manual to find out which form is used for each
setting.
Martin Geisler - Aug. 8, 2013, 7:23 a.m.
Matt Mackall <mpm@selenic.com> writes:

> On Wed, 2013-08-07 at 13:31 -0400, Augie Fackler wrote:
>> On Wed, Aug 07, 2013 at 06:06:53PM +0200, Martin Geisler wrote:
>> > Kevin Bullock <kbullock+mercurial@ringworld.org> writes:
>> >
>> > > On 2 Aug 2013, at 9:15 AM, Augie Fackler wrote:
>> > >
>> > >> # HG changeset patch
>> > >> # User Augie Fackler <durin42@gmail.com>
>> > >> # Date 1374709814 14400
>> > >> #      Wed Jul 24 19:50:14 2013 -0400
>> > >> # Node ID d59a05f3c4e013ecf9c7d3b40c04fa9900b7f74f
>> > >> # Parent  4c9c2538d46bed89734a465ae01dbda483154425
>> > >> push: allow specifying default-push as defaultpush
>> > >
>> > > Unlike the rest of the series, this one causes us to change the output
>> > > -- on one of our most well-publicized config options. I guess I'm -1
>> > > on this until we're ready to officially favor the
>> > > non-hyphenated/-underscored forms. Rest of the series looks fine to
>> > > me.
>> >
>> > Why would you even prefer the non-hyphenated versions? I know we use
>> > names without underscores in the Python code, but I think someone needs
>> > to explain how the arguments for that carry over to the config keys.
>
> The argument has never been "underscores are bad", it's always been
> "consistency is good". And that argument obviously applies to config
> keys just as well as source code identifiers.

Of course, I don't think anybody has argued against consistency. My
question was rather if the audience for the config files have the same
requirements as the audience for our Python code.

That is, one might argue that the identifiers in the code are
cross-referenced more than the config keys.

Writing a config file in the first place is also an exercise in
cross-referencing, but I have the impression that people edit their
config files with the manual in hand anyway -- you search for a config
key based on the description of what it does and then you copy the key
to your config file.

This is slightly different from how we write code: there we repeatedly
use the same identifier and hope to avoid double-checking if it's called
LocalRepository, local_repository, or localrepository.

That was the distinction I tried to point out.

> "How do I fix this?"
>
> "You just need to set default push."
>
> "Is that default no space push, default underscore push, or default dash
> push?"
>
> "Default dash push. Then set allow push and allow pull on the server."
>
> "They don't do anything?!"
>
> "Oh, sorry, that's allow underscore push and allow no space pull."
>
> "Are you freaking kidding me?"
>
>
> I'm not particularly attached to a particular choice of style, just
> that _there is a style_ and people (like me, fercrissake) don't need
> to repeatedly consult the manual to find out which form is used for
> each setting.

Yes, it is important that there is a consistent style. A style without
spaces is probably easiest at this point.
Martin Geisler - Aug. 8, 2013, 7:42 a.m.
Augie Fackler <raf@durin42.com> writes:

> On Wed, Aug 07, 2013 at 06:06:53PM +0200, Martin Geisler wrote:
>> Kevin Bullock <kbullock+mercurial@ringworld.org> writes:
>>
>> > On 2 Aug 2013, at 9:15 AM, Augie Fackler wrote:
>> >
>> >> # HG changeset patch
>> >> # User Augie Fackler <durin42@gmail.com>
>> >> # Date 1374709814 14400
>> >> #      Wed Jul 24 19:50:14 2013 -0400
>> >> # Node ID d59a05f3c4e013ecf9c7d3b40c04fa9900b7f74f
>> >> # Parent  4c9c2538d46bed89734a465ae01dbda483154425
>> >> push: allow specifying default-push as defaultpush
>> >
>> > Unlike the rest of the series, this one causes us to change the output
>> > -- on one of our most well-publicized config options. I guess I'm -1
>> > on this until we're ready to officially favor the
>> > non-hyphenated/-underscored forms. Rest of the series looks fine to
>> > me.
>>
>> Why would you even prefer the non-hyphenated versions? I know we use
>> names without underscores in the Python code, but I think someone needs
>> to explain how the arguments for that carry over to the config keys.
>>
>> I think that having no underscores or hyphens comes at a cost in
>> readability and our docs might look more friendly if people see
>>
>>   [paths]
>>   default-push = http://...
>>
>> instead of the version without a hyphen. I realize, though, that we have
>> a lot more options with run-together words.
>
> As we add configuration options, they will be without dashes or
> underscores, and it'd be nice to slowly converge on a single format
> instead of the patchwork quilt we have now.

I understood that it's nice to make the options consistent.

What I tried to ask is why you prefer the config keys without a
separator (hyphen/underscore)? In other words, I was hoping that someone
would explain why 'defaultpush' is a better config key than the current
'default-push'.

Arguing that 95% of our config keys use no separator would be such an
argument.

Note that we have come conflicts:

  pre-commit, precommit
  pre-outgoing, preoutgoing
  pre-tag, pretag
  pre-update, preupdate

We might want to add a new name for the 'prefoo' keys, maybe like
'control-foo' or something like that (we'll of course keep parsing the
old 'prefoo' key, but we can drop it from the documentation).

Patch

diff --git a/hgext/largefiles/basestore.py b/hgext/largefiles/basestore.py
--- a/hgext/largefiles/basestore.py
+++ b/hgext/largefiles/basestore.py
@@ -173,10 +173,10 @@ 
         else:
             path = ui.expandpath('default-push', 'default')
 
-        # ui.expandpath() leaves 'default-push' and 'default' alone if
-        # they cannot be expanded: fallback to the empty string,
-        # meaning the current directory.
-        if path == 'default-push' or path == 'default':
+        # ui.expandpath() leaves 'defaultpush', 'default-push' and
+        # 'default' alone if they cannot be expanded: fallback to the
+        # empty string, meaning the current directory.
+        if path in ('default-push', 'defaultpush', 'default'):
             path = ''
             remote = repo
         else:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4403,14 +4403,14 @@ 
     configuration file and in ``/etc/mercurial/hgrc``. If run inside a
     repository, ``.hg/hgrc`` is used, too.
 
-    The path names ``default`` and ``default-push`` have a special
+    The path names ``default`` and ``defaultpush`` have a special
     meaning.  When performing a push or pull operation, they are used
     as fallbacks if no location is specified on the command-line.
-    When ``default-push`` is set, it will be used for push and
+    When ``defaultpush`` is set, it will be used for push and
     ``default`` will be used for pull; otherwise ``default`` is used
     as the fallback for both.  When cloning a repository, the clone
     source is written as ``default`` in ``.hg/hgrc``.  Note that
-    ``default`` and ``default-push`` apply to all inbound (e.g.
+    ``default`` and ``defaultpush`` apply to all inbound (e.g.
     :hg:`incoming`) and outbound (e.g. :hg:`outgoing`, :hg:`email` and
     :hg:`bundle`) operations.
 
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -285,8 +285,8 @@ 
     else: # recursion reached top repo
         if util.safehasattr(repo, '_subtoppath'):
             return repo._subtoppath
-        if push and repo.ui.config('paths', 'default-push'):
-            return repo.ui.config('paths', 'default-push')
+        if push and repo.ui.config('paths', ['defaultpush', 'default-push']):
+            return repo.ui.config('paths', ['defaultpush', 'default-push'])
         if repo.ui.config('paths', 'default'):
             return repo.ui.config('paths', 'default')
         if repo.sharedpath != repo.path:
@@ -543,7 +543,7 @@ 
             defpushpath = _abssource(self._repo, True, abort=False)
             addpathconfig('default', defpath)
             if defpath != defpushpath:
-                addpathconfig('default-push', defpushpath)
+                addpathconfig('defaultpush', defpushpath)
             fp.close()
 
     @annotatesubrepoerror
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -465,9 +465,13 @@ 
         if util.hasscheme(loc) or os.path.isdir(os.path.join(loc, '.hg')):
             return loc
 
+        if loc in ('defaultpush', 'default-push'):
+            loc = ['defaultpush', 'default-push']
         path = self.config('paths', loc)
         if not path and default is not None:
             path = self.config('paths', default)
+        if isinstance(loc, list):
+            loc = loc[0]
         return path or loc
 
     def pushbuffer(self):
diff --git a/tests/test-bundle-r.t b/tests/test-bundle-r.t
--- a/tests/test-bundle-r.t
+++ b/tests/test-bundle-r.t
@@ -162,7 +162,7 @@ 
   abort: --base is incompatible with specifying a destination
   [255]
   $ hg -R test bundle -r tip test-bundle-branch1.hg
-  abort: repository default-push not found!
+  abort: repository defaultpush not found!
   [255]
 
   $ hg -R test bundle --base 2 -r tip test-bundle-branch1.hg