Patchwork [RFC] commands: add 'setup' command to allow automatic hgrc configuration

login
register
mail settings
Submitter Mathias De Maré
Date March 12, 2015, 5:24 p.m.
Message ID <0de0fc66a20a5f0658a6.1426181089@mathias-Latitude-E6540>
Download mbox | patch
Permalink /patch/8027/
State Superseded
Headers show

Comments

Mathias De Maré - March 12, 2015, 5:24 p.m.
# HG changeset patch
# User Mathias De Maré <mathias.demare@gmail.com>
# Date 1426162870 -3600
#      Thu Mar 12 13:21:10 2015 +0100
# Node ID 0de0fc66a20a5f0658a6c6f1f5de20d4705f3de1
# Parent  7cf9a9e0cf893e7ae82dc576a03c843fd6640438
commands: add 'setup' command to allow automatic hgrc configuration

This is a proposal meant to easily set up new users with
a good hgrc file. It's very basic (a combination of
'release early' and 'I don't have a lot of time').
Comments are very much welcome.

I wasn't too sure about using the first path
listed in the userhgrc, not sure if that's the best approach.

Some other ideas to throw out there:
- Running 'setup' by default if a user does not have a .hgrc
      and is using the terminal (possibly falls under moot?)
- Perhaps having a global 'do you want to have a lot of sensible defaults set'
  question could be useful as well.
- Popping up a merge window in case a user already has a .hgrc.
  This could allow the user to do the merge,
  and avoids us having to mess with editing configs
  (this was a very good idea by smf).
Mathias De Maré - March 12, 2015, 6 p.m.
On Thu, Mar 12, 2015 at 6:24 PM, <mathias.demare@gmail.com> wrote:

> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1426162870 -3600
> #      Thu Mar 12 13:21:10 2015 +0100
> # Node ID 0de0fc66a20a5f0658a6c6f1f5de20d4705f3de1
> # Parent  7cf9a9e0cf893e7ae82dc576a03c843fd6640438
> commands: add 'setup' command to allow automatic hgrc configuration
>
I'm terribly sorry about this, I forgot to amend my commit. Today's just
not my day :-/

>
> This is a proposal meant to easily set up new users with
> a good hgrc file. It's very basic (a combination of
> 'release early' and 'I don't have a lot of time').
> Comments are very much welcome.
>
> I wasn't too sure about using the first path
> listed in the userhgrc, not sure if that's the best approach.
>
> Some other ideas to throw out there:
> - Running 'setup' by default if a user does not have a .hgrc
>       and is using the terminal (possibly falls under moot?)
> - Perhaps having a global 'do you want to have a lot of sensible defaults
> set'
>   question could be useful as well.
> - Popping up a merge window in case a user already has a .hgrc.
>   This could allow the user to do the merge,
>   and avoids us having to mess with editing configs
>   (this was a very good idea by smf).
>
> diff -r 7cf9a9e0cf89 -r 0de0fc66a20a mercurial/commands.py
> --- a/mercurial/commands.py     Wed Mar 11 15:22:34 2015 -0700
> +++ b/mercurial/commands.py     Thu Mar 12 13:21:10 2015 +0100
> @@ -19,10 +19,12 @@
>  import merge as mergemod
>  import minirst, revset, fileset
>  import dagparser, context, simplemerge, graphmod, copies
> +import query
>  import random
>  import setdiscovery, treediscovery, dagutil, pvec, localrepo
>  import phases, obsolete, exchange, bundle2
>  import ui as uimod
> +import config as configmod
>
>  table = {}
>
> @@ -5615,6 +5617,50 @@
>          self.httpd.serve_forever()
>
>
> +@command('setup', [])
> +def setup(ui, repo):
> +    """run a setup to generate a Mercurial configuration
> +
> +    Answer a few questions to generate a basic Mercurial configuration.
> +
> +    Returns 0 on success.
> +    """
> +
> +    paths = scmutil.userrcpath()
> +    if os.path.isfile(paths[0]):
> +        if not query.binaryquery(repo,
> +                _('WARNING: you already have a ' \
> +                'Mercurial configuration file!\n' \
> +                'Are you sure you want to overwrite it ' \
> +                'with a new configuration?'),
> +            False):
> +            return
> +
> +    userconfig = configmod.config()
> +
> +    if query.binaryquery(repo,
> +            _('Would you like to configure your username? '), True):
> +        name = query.textquery(repo, _('Please enter your name: '))
> +        email = query.textquery(repo, _('Please enter your email adress:
> '))
> +        userconfig.set('ui', 'username', '%s <%s>' % (name, email))
> +    if query.binaryquery(repo, _('Would you like to enable ' \
> +            'some useful extensions by default, ' \
> +            'enhancing your experience?\n' \
> +            'The extensions to be enabled are: ' \
> +            'color, pager, progress'), True):
> +        userconfig.set('extensions', 'color', '')
> +        userconfig.set('extensions', 'pager', '')
> +        userconfig.set('pager', 'pager', 'less -FRX')
> +        userconfig.set('extensions', 'progress', '')
> +    if query.binaryquery(repo,
> +            'Would you like to use an extended diff format? ' \
> +            'If you are viewing changes, ' \
> +            'this allows you to easily see added/removed files.',
> +            True):
> +        userconfig.set('diff', 'git', True)
> +    userconfig.write(paths[0])
> +
> +
>  @command('^status|st',
>      [('A', 'all', None, _('show status of all files')),
>      ('m', 'modified', None, _('show only modified files')),
> diff -r 7cf9a9e0cf89 -r 0de0fc66a20a mercurial/config.py
> --- a/mercurial/config.py       Wed Mar 11 15:22:34 2015 -0700
> +++ b/mercurial/config.py       Thu Mar 12 13:21:10 2015 +0100
> @@ -157,3 +157,13 @@
>          if not fp:
>              fp = util.posixfile(path)
>          self.parse(path, fp.read(), sections, remap, self.read)
> +
> +    def write(self, path, fp=None):
> +        if not fp:
> +            fp = util.posixfile(path, 'w')
> +        for section in self:
> +            d = self[section]
> +            fp.write('[%s]\n' % section)
> +            for key in d:
> +                fp.write('%s = %s\n' % (key, d[key]))
> +            fp.write('\n')
> diff -r 7cf9a9e0cf89 -r 0de0fc66a20a mercurial/query.py
> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> +++ b/mercurial/query.py        Thu Mar 12 13:21:10 2015 +0100
> @@ -0,0 +1,42 @@
> +# query.py - user querying for mercurial
> +#
> +# Copyright 2015 Mathias De Maré <mathias.demare@gmail.com>
> +#
> +# This software may be used and distributed according to the terms of the
> +# GNU General Public License version 2 or any later version.
> +
> +def _generateresponsestr(responses, default):
> +    defaultidx = responses.index(default)
> +    if defaultidx >= 0 and len(responses[defaultidx]) > 0:
> +        responses[defaultidx] = responses[defaultidx].title()
> +    return '(%s)' % '/'.join(responses)
> +
> +def textquery(repo, question, emptyallowed=False):
> +    """Query the user using an open question"""
> +    repo.ui.write('%s\n' % question)
> +    while True:
> +        res = raw_input()
> +        if emptyallowed or len(res):
> +            return res
> +
> +def optionquery(repo, question, responses, default):
> +    """Query the user using question and a given set of parameters.
> +    Once a valid response is given, the query returns
> +    the match from the list of responses."""
> +    repo.ui.write('%s %s\n' % (question, _generateresponsestr(responses,
> default)))
> +    while True:
> +        res = raw_input()
> +        if not len(res) and default:
> +            return default
> +        for response in responses:
> +            if response.startswith(res):
> +                return response
> +
> +def binaryquery(repo, question, default=True):
> +    """Query the user using question and 'yes/no'"""
> +    if default:
> +        default = 'yes'
> +    else:
> +        default = 'no'
> +    res = optionquery(repo, question, ['yes', 'no'], default)
> +    return res == 'yes'
>
Matt Mackall - March 12, 2015, 6:30 p.m.
On Thu, 2015-03-12 at 18:24 +0100, mathias.demare@gmail.com wrote:
> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1426162870 -3600
> #      Thu Mar 12 13:21:10 2015 +0100
> # Node ID 0de0fc66a20a5f0658a6c6f1f5de20d4705f3de1
> # Parent  7cf9a9e0cf893e7ae82dc576a03c843fd6640438
> commands: add 'setup' command to allow automatic hgrc configuration

I'm not terribly excited about this. It just delays people learning how
to edit their config file which is not that hard and much more useful in
the long run.

> I wasn't too sure about using the first path
> listed in the userhgrc, not sure if that's the best approach.

It should agree with whatever "hg conf -e" does when no config exists.
But maybe you don't know about that?

> - Popping up a merge window in case a user already has a .hgrc.
>   This could allow the user to do the merge,
>   and avoids us having to mess with editing configs
>   (this was a very good idea by smf).

Terrifying. I'm the type of user who's intimidated by editing a config
file.. so you're going to launch into a program that shows me four
different copies of that file all at once? I don't see how that's
helping.

> +@command('setup', [])
> +def setup(ui, repo):
> +    """run a setup to generate a Mercurial configuration
> +
> +    Answer a few questions to generate a basic Mercurial configuration.
> +
> +    Returns 0 on success.
> +    """
> +
> +    paths = scmutil.userrcpath()
> +    if os.path.isfile(paths[0]):
> +        if not query.binaryquery(repo,
> +                _('WARNING: you already have a ' \
> +                'Mercurial configuration file!\n' \
> +                'Are you sure you want to overwrite it ' \
> +                'with a new configuration?'),
> +            False):
> +            return

You seem to be inventing a wholly-new set of ways of warning and
prompting the user when we already have ui.prompt* and ui.warn?

And for some reason, they take a repo argument rather than a ui
argument?

> +    if query.binaryquery(repo,
> +            _('Would you like to configure your username? '), True):
> +        name = query.textquery(repo, _('Please enter your name: '))

FWIW, Mercurial message style is to use lowercase.

> +        userconfig.set('pager', 'pager', 'less -FRX')

There's a reason this isn't the default in the pager extension, which is
that there's already a standard way to set your preferred pager and
options across applications that we should respect.

> +        userconfig.set('extensions', 'progress', '')
> +    if query.binaryquery(repo,
> +            'Would you like to use an extended diff format? ' \
> +            'If you are viewing changes, ' \
> +            'this allows you to easily see added/removed files.',

"..but will break compatibility with standard diff/patch tools."

Again, there's a reason it's not on by default. And generally speaking,
diverging from orthodoxy in minor details won't do your main cause any
favors.
Mathias De Maré - March 12, 2015, 6:53 p.m.
On Thu, Mar 12, 2015 at 7:30 PM, Matt Mackall <mpm@selenic.com> wrote:

> On Thu, 2015-03-12 at 18:24 +0100, mathias.demare@gmail.com wrote:
> > # HG changeset patch
> > # User Mathias De Maré <mathias.demare@gmail.com>
> > # Date 1426162870 -3600
> > #      Thu Mar 12 13:21:10 2015 +0100
> > # Node ID 0de0fc66a20a5f0658a6c6f1f5de20d4705f3de1
> > # Parent  7cf9a9e0cf893e7ae82dc576a03c843fd6640438
> > commands: add 'setup' command to allow automatic hgrc configuration
>
> I'm not terribly excited about this. It just delays people learning how
> to edit their config file which is not that hard and much more useful in
> the long run.
>
I agree that users do need to learn that in the long run. I was mostly
thinking about this towards first impressions for users, and to allow them
to enable some useful items which they otherwise don't know exist.
I'd like to find a way to make the user experience for new users as good as
possible.

Each of your comments below raises a good point, I'll need to fix those
things. However, is it useful in general to continue with this (seeing as
you're indeed not too enthousiastic about this), or should I just drop it?

>
> > I wasn't too sure about using the first path
> > listed in the userhgrc, not sure if that's the best approach.
>
> It should agree with whatever "hg conf -e" does when no config exists.
> But maybe you don't know about that?
>
Oh, good point, I should've looked there. Perhaps showing the result at the
end (like with 'hg conf -e') would be useful as well.

>
> > - Popping up a merge window in case a user already has a .hgrc.
> >   This could allow the user to do the merge,
> >   and avoids us having to mess with editing configs
> >   (this was a very good idea by smf).
>
> Terrifying. I'm the type of user who's intimidated by editing a config
> file.. so you're going to launch into a program that shows me four
> different copies of that file all at once? I don't see how that's
> helping.
>
Again a good point, perhaps not a good thing to implement after all.

>
> > +@command('setup', [])
> > +def setup(ui, repo):
> > +    """run a setup to generate a Mercurial configuration
> > +
> > +    Answer a few questions to generate a basic Mercurial configuration.
> > +
> > +    Returns 0 on success.
> > +    """
> > +
> > +    paths = scmutil.userrcpath()
> > +    if os.path.isfile(paths[0]):
> > +        if not query.binaryquery(repo,
> > +                _('WARNING: you already have a ' \
> > +                'Mercurial configuration file!\n' \
> > +                'Are you sure you want to overwrite it ' \
> > +                'with a new configuration?'),
> > +            False):
> > +            return
>
> You seem to be inventing a wholly-new set of ways of warning and
> prompting the user when we already have ui.prompt* and ui.warn?
>
> And for some reason, they take a repo argument rather than a ui
> argument?
>
Yes, I made a bad call here :-/ I'll need to change this.

>
> > +    if query.binaryquery(repo,
> > +            _('Would you like to configure your username? '), True):
> > +        name = query.textquery(repo, _('Please enter your name: '))
>
> FWIW, Mercurial message style is to use lowercase.
>
> > +        userconfig.set('pager', 'pager', 'less -FRX')
>
> There's a reason this isn't the default in the pager extension, which is
> that there's already a standard way to set your preferred pager and
> options across applications that we should respect.
>
> > +        userconfig.set('extensions', 'progress', '')
> > +    if query.binaryquery(repo,
> > +            'Would you like to use an extended diff format? ' \
> > +            'If you are viewing changes, ' \
> > +            'this allows you to easily see added/removed files.',
>
> "..but will break compatibility with standard diff/patch tools."
>
> Again, there's a reason it's not on by default. And generally speaking,
> diverging from orthodoxy in minor details won't do your main cause any
> favors.
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>
Matt Mackall - March 12, 2015, 7:20 p.m.
On Thu, 2015-03-12 at 19:53 +0100, Mathias De Maré wrote:
> On Thu, Mar 12, 2015 at 7:30 PM, Matt Mackall <mpm@selenic.com> wrote:
> 
> > On Thu, 2015-03-12 at 18:24 +0100, mathias.demare@gmail.com wrote:
> > > # HG changeset patch
> > > # User Mathias De Maré <mathias.demare@gmail.com>
> > > # Date 1426162870 -3600
> > > #      Thu Mar 12 13:21:10 2015 +0100
> > > # Node ID 0de0fc66a20a5f0658a6c6f1f5de20d4705f3de1
> > > # Parent  7cf9a9e0cf893e7ae82dc576a03c843fd6640438
> > > commands: add 'setup' command to allow automatic hgrc configuration
> >
> > I'm not terribly excited about this. It just delays people learning how
> > to edit their config file which is not that hard and much more useful in
> > the long run.
> >
> I agree that users do need to learn that in the long run. I was mostly
> thinking about this towards first impressions for users, and to allow them
> to enable some useful items which they otherwise don't know exist.
> I'd like to find a way to make the user experience for new users as good as
> possible.

> > Terrifying. I'm the type of user who's intimidated by editing a config
> > file.. so you're going to launch into a program that shows me four
> > different copies of that file all at once? I don't see how that's
> > helping.
> >
> Again a good point, perhaps not a good thing to implement after all.

Yeah, I don't think there's a good second-use story here. This creates a
problem for the whole setup idea though: if I use setup once, I'm now
stranded and need to learn/discover a whole new thing to edit my config.

I think we should focus on the conf -e experience. If we want to alias
that to setup, I might be behind that, since it's a more-than-one-use
thing. I actually find myself using conf -e a lot even though it's no
harder to type than 'emacs ~/.hgrc'.

Patch

diff -r 7cf9a9e0cf89 -r 0de0fc66a20a mercurial/commands.py
--- a/mercurial/commands.py	Wed Mar 11 15:22:34 2015 -0700
+++ b/mercurial/commands.py	Thu Mar 12 13:21:10 2015 +0100
@@ -19,10 +19,12 @@ 
 import merge as mergemod
 import minirst, revset, fileset
 import dagparser, context, simplemerge, graphmod, copies
+import query
 import random
 import setdiscovery, treediscovery, dagutil, pvec, localrepo
 import phases, obsolete, exchange, bundle2
 import ui as uimod
+import config as configmod
 
 table = {}
 
@@ -5615,6 +5617,50 @@ 
         self.httpd.serve_forever()
 
 
+@command('setup', [])
+def setup(ui, repo):
+    """run a setup to generate a Mercurial configuration
+
+    Answer a few questions to generate a basic Mercurial configuration.
+
+    Returns 0 on success.
+    """
+
+    paths = scmutil.userrcpath()
+    if os.path.isfile(paths[0]):
+        if not query.binaryquery(repo,
+                _('WARNING: you already have a ' \
+                'Mercurial configuration file!\n' \
+                'Are you sure you want to overwrite it ' \
+                'with a new configuration?'),
+            False):
+            return
+
+    userconfig = configmod.config()
+
+    if query.binaryquery(repo,
+            _('Would you like to configure your username? '), True):
+        name = query.textquery(repo, _('Please enter your name: '))
+        email = query.textquery(repo, _('Please enter your email adress: '))
+        userconfig.set('ui', 'username', '%s <%s>' % (name, email))
+    if query.binaryquery(repo, _('Would you like to enable ' \
+            'some useful extensions by default, ' \
+            'enhancing your experience?\n' \
+            'The extensions to be enabled are: ' \
+            'color, pager, progress'), True):
+        userconfig.set('extensions', 'color', '')
+        userconfig.set('extensions', 'pager', '')
+        userconfig.set('pager', 'pager', 'less -FRX')
+        userconfig.set('extensions', 'progress', '')
+    if query.binaryquery(repo,
+            'Would you like to use an extended diff format? ' \
+            'If you are viewing changes, ' \
+            'this allows you to easily see added/removed files.',
+            True):
+        userconfig.set('diff', 'git', True)
+    userconfig.write(paths[0])
+
+
 @command('^status|st',
     [('A', 'all', None, _('show status of all files')),
     ('m', 'modified', None, _('show only modified files')),
diff -r 7cf9a9e0cf89 -r 0de0fc66a20a mercurial/config.py
--- a/mercurial/config.py	Wed Mar 11 15:22:34 2015 -0700
+++ b/mercurial/config.py	Thu Mar 12 13:21:10 2015 +0100
@@ -157,3 +157,13 @@ 
         if not fp:
             fp = util.posixfile(path)
         self.parse(path, fp.read(), sections, remap, self.read)
+
+    def write(self, path, fp=None):
+        if not fp:
+            fp = util.posixfile(path, 'w')
+        for section in self:
+            d = self[section]
+            fp.write('[%s]\n' % section)
+            for key in d:
+                fp.write('%s = %s\n' % (key, d[key]))
+            fp.write('\n')
diff -r 7cf9a9e0cf89 -r 0de0fc66a20a mercurial/query.py
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/mercurial/query.py	Thu Mar 12 13:21:10 2015 +0100
@@ -0,0 +1,42 @@ 
+# query.py - user querying for mercurial
+#
+# Copyright 2015 Mathias De Maré <mathias.demare@gmail.com>
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+def _generateresponsestr(responses, default):
+    defaultidx = responses.index(default)
+    if defaultidx >= 0 and len(responses[defaultidx]) > 0:
+        responses[defaultidx] = responses[defaultidx].title()
+    return '(%s)' % '/'.join(responses)
+
+def textquery(repo, question, emptyallowed=False):
+    """Query the user using an open question"""
+    repo.ui.write('%s\n' % question)
+    while True:
+        res = raw_input()
+        if emptyallowed or len(res):
+            return res
+
+def optionquery(repo, question, responses, default):
+    """Query the user using question and a given set of parameters.
+    Once a valid response is given, the query returns
+    the match from the list of responses."""
+    repo.ui.write('%s %s\n' % (question, _generateresponsestr(responses, default)))
+    while True:
+        res = raw_input()
+        if not len(res) and default:
+            return default
+        for response in responses:
+            if response.startswith(res):
+                return response
+
+def binaryquery(repo, question, default=True):
+    """Query the user using question and 'yes/no'"""
+    if default:
+        default = 'yes'
+    else:
+        default = 'no'
+    res = optionquery(repo, question, ['yes', 'no'], default)
+    return res == 'yes'