Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-08 Thread Jun Wu
A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
defining "system defaults". A new section also makes it harder to see what
configs they are overriding.

How about appending ":sysdefault" to normal configs? The idea was also
mentioned by Yuya at [1] and we use them in "[paths]":

  [ui]
  pager:sysdefault = sensible-pager
  editor:sysdefault = sensible-editor

This makes it easier to see what configs they are overriding, and makes the
section less crowded if we want the same thing for other sections.

[1]: https://patchwork.mercurial-scm.org/patch/16825/

Excerpts from Augie Fackler's message of 2017-03-08 18:48:55 -0500:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1489016567 18000
> #  Wed Mar 08 18:42:47 2017 -0500
> # Node ID 71fc64c48cfff1b7a2120c60e2b958da3263c0dc
> # Parent  92f7d6585c185e85763b3bad81b1304b8cdb5937
> ui: introduce sysdefault section for pager and editor configuration
> 
> The debian package currently has to patch Mercurial to move the
> default editor from `vi` to `sensible-editor`. Now that we're growing
> another suboptimal-on-most-platforms default program (`more` as the
> pager), let's do packagers a small favor and give them a place where
> they can specify the default program for their platform, rather than
> having to rely on patching code during the build process. I'd expect
> the configuration on OS X to be something like:
> 
> [sysdefault]
> editor = nano
> pager = LESS=FRX less
> 
> and on debian to be:
> 
> [sysdefault]
> editor = sensible-editor
> pager = sensible-pager
> 
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -907,13 +907,11 @@ class ui(object):
>  # HGPLAINEXCEPT=pager, and the user didn't specify --debug.
>  return
>  
> -# TODO: add a "system defaults" config section so this default
> -# of more(1) can be easily replaced with a global
> -# configuration file. For example, on OS X the sane default is
> -# less(1), not more(1), and on debian it's
> -# sensible-pager(1). We should probably also give the system
> -# default editor command similar treatment.
> -envpager = encoding.environ.get('PAGER', 'more')
> +# sysdefault.pager is available for packagers or system
> +# administrators to specify a saner default pager for their
> +# environment.
> +defaultpager = self.config('sysdefault', 'pager', default='more')
> +envpager = encoding.environ.get('PAGER', defaultpager)
>  pagercmd = self.config('pager', 'pager', envpager)
>  if not pagercmd:
>  return
> @@ -1348,6 +1346,10 @@ class ui(object):
>  editor = 'E'
>  else:
>  editor = 'vi'
> +# sysdefault.editor is available for packagers or system
> +# administrators to specify a saner default editor for their
> +# environment.
> +editor = self.config("sysdefault", "editor", default=editor)
>  return (encoding.environ.get("HGEDITOR") or
>  self.config("ui", "editor") or
>  encoding.environ.get("VISUAL") or
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-09 Thread Yuya Nishihara
On Wed, 8 Mar 2017 16:06:12 -0800, Jun Wu wrote:
> A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
> defining "system defaults". A new section also makes it harder to see what
> configs they are overriding.
> 
> How about appending ":sysdefault" to normal configs? The idea was also
> mentioned by Yuya at [1] and we use them in "[paths]":
> 
>   [ui]
>   pager:sysdefault = sensible-pager
>   editor:sysdefault = sensible-editor
> 
> This makes it easier to see what configs they are overriding, and makes the
> section less crowded if we want the same thing for other sections.
> 
> [1]: https://patchwork.mercurial-scm.org/patch/16825/

A separate section sounds good to me. It would serve a similar role to
[merge-tools], which normal users wouldn't touch. I think :attr syntax
is more open to users.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-09 Thread Kevin Bullock
> On Mar 8, 2017, at 18:06, Jun Wu  wrote:
> 
> A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
> defining "system defaults". A new section also makes it harder to see what
> configs they are overriding.
> 
> How about appending ":sysdefault" to normal configs? The idea was also
> mentioned by Yuya at [1] and we use them in "[paths]":
> 
>  [ui]
>  pager:sysdefault = sensible-pager
>  editor:sysdefault = sensible-editor
> 
> This makes it easier to see what configs they are overriding, and makes the
> section less crowded if we want the same thing for other sections.

I like that idea.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-09 Thread Sean Farley
Kevin Bullock  writes:

>> On Mar 8, 2017, at 18:06, Jun Wu  wrote:
>> 
>> A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
>> defining "system defaults". A new section also makes it harder to see what
>> configs they are overriding.
>> 
>> How about appending ":sysdefault" to normal configs? The idea was also
>> mentioned by Yuya at [1] and we use them in "[paths]":
>> 
>>  [ui]
>>  pager:sysdefault = sensible-pager
>>  editor:sysdefault = sensible-editor
>> 
>> This makes it easier to see what configs they are overriding, and makes the
>> section less crowded if we want the same thing for other sections.
>
> I like that idea.

I guess I don't follow since I think of things in /etc as the system
defaults to begin with. Are we having a namespace clash with users
having 'foo=BAR' set so that setting 'foo' in /etc won't override that?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-09 Thread Jun Wu
Excerpts from Sean Farley's message of 2017-03-09 18:49:18 -0800:
> Kevin Bullock  writes:
> 
> >> On Mar 8, 2017, at 18:06, Jun Wu  wrote:
> >> 
> >> A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
> >> defining "system defaults". A new section also makes it harder to see what
> >> configs they are overriding.
> >> 
> >> How about appending ":sysdefault" to normal configs? The idea was also
> >> mentioned by Yuya at [1] and we use them in "[paths]":
> >> 
> >>  [ui]
> >>  pager:sysdefault = sensible-pager
> >>  editor:sysdefault = sensible-editor
> >> 
> >> This makes it easier to see what configs they are overriding, and makes the
> >> section less crowded if we want the same thing for other sections.
> >
> > I like that idea.
> 
> I guess I don't follow since I think of things in /etc as the system
> defaults to begin with. Are we having a namespace clash with users
> having 'foo=BAR' set so that setting 'foo' in /etc won't override that?

The point is to define configs that cannot be overrided by environment
variables. If we check "source", and treat things in "/etc" differently, we
may get rid of this. But that may make the config system more complicated.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-09 Thread Jun Wu
Excerpts from Jun Wu's message of 2017-03-09 18:52:34 -0800:
> Excerpts from Sean Farley's message of 2017-03-09 18:49:18 -0800:
> > Kevin Bullock  writes:
> > 
> > >> On Mar 8, 2017, at 18:06, Jun Wu  wrote:
> > >> 
> > >> A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
> > >> defining "system defaults". A new section also makes it harder to see 
> > >> what
> > >> configs they are overriding.
> > >> 
> > >> How about appending ":sysdefault" to normal configs? The idea was also
> > >> mentioned by Yuya at [1] and we use them in "[paths]":
> > >> 
> > >>  [ui]
> > >>  pager:sysdefault = sensible-pager
> > >>  editor:sysdefault = sensible-editor
> > >> 
> > >> This makes it easier to see what configs they are overriding, and makes 
> > >> the
> > >> section less crowded if we want the same thing for other sections.
> > >
> > > I like that idea.
> > 
> > I guess I don't follow since I think of things in /etc as the system
> > defaults to begin with. Are we having a namespace clash with users
> > having 'foo=BAR' set so that setting 'foo' in /etc won't override that?
> 
> The point is to define configs that cannot be overrided by environment
> variables. If we check "source", and treat things in "/etc" differently, we
> may get rid of this. But that may make the config system more complicated.

I forgot to mention I have a half-complete series that changes "config" to
layered, immutable config objects that are chainned together. So we can
figure out layers of configs easily. However, that also introduces some
overhead, especially when we want to maintain correct order of config items.

If we decide to go the modern way of having layered immutable configs, at
the cost of scarifying perf or memory usage, I can polish and send that
series.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-09 Thread Sean Farley
Jun Wu  writes:

> Excerpts from Sean Farley's message of 2017-03-09 18:49:18 -0800:
>> Kevin Bullock  writes:
>> 
>> >> On Mar 8, 2017, at 18:06, Jun Wu  wrote:
>> >> 
>> >> A "sysdefault" section sounds a bit weird. Usually files under "/etc" are
>> >> defining "system defaults". A new section also makes it harder to see what
>> >> configs they are overriding.
>> >> 
>> >> How about appending ":sysdefault" to normal configs? The idea was also
>> >> mentioned by Yuya at [1] and we use them in "[paths]":
>> >> 
>> >>  [ui]
>> >>  pager:sysdefault = sensible-pager
>> >>  editor:sysdefault = sensible-editor
>> >> 
>> >> This makes it easier to see what configs they are overriding, and makes 
>> >> the
>> >> section less crowded if we want the same thing for other sections.
>> >
>> > I like that idea.
>> 
>> I guess I don't follow since I think of things in /etc as the system
>> defaults to begin with. Are we having a namespace clash with users
>> having 'foo=BAR' set so that setting 'foo' in /etc won't override that?
>
> The point is to define configs that cannot be overrided by environment
> variables. If we check "source", and treat things in "/etc" differently, we
> may get rid of this. But that may make the config system more complicated.

Ok, I see.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH RFC] ui: introduce sysdefault section for pager and editor configuration

2017-03-12 Thread Jun Wu
What if instead of reading environments directly, introduce a new config
layer which is converted from environment variables? Like:

  - (Top)layer: command line config flags
  -  layer: user configs
  -  layer: config converted from environment variables
like, convert "HGEDITOR" to "ui.editor"
  - (Bottom) layer: system configs

In this way, we don't need a [sysdefault] and can just put things in /etc.
And that will solve other problems like:

   ryanmce's complaint to me yesterday was HGEDITOR=vim hg
--config ui.editor=true foo   would run vim, even though editor is
'specified on command line'...

Excerpts from Augie Fackler's message of 2017-03-08 18:48:55 -0500:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1489016567 18000
> #  Wed Mar 08 18:42:47 2017 -0500
> # Node ID 71fc64c48cfff1b7a2120c60e2b958da3263c0dc
> # Parent  92f7d6585c185e85763b3bad81b1304b8cdb5937
> ui: introduce sysdefault section for pager and editor configuration
> 
> The debian package currently has to patch Mercurial to move the
> default editor from `vi` to `sensible-editor`. Now that we're growing
> another suboptimal-on-most-platforms default program (`more` as the
> pager), let's do packagers a small favor and give them a place where
> they can specify the default program for their platform, rather than
> having to rely on patching code during the build process. I'd expect
> the configuration on OS X to be something like:
> 
> [sysdefault]
> editor = nano
> pager = LESS=FRX less
> 
> and on debian to be:
> 
> [sysdefault]
> editor = sensible-editor
> pager = sensible-pager
> 
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -907,13 +907,11 @@ class ui(object):
>  # HGPLAINEXCEPT=pager, and the user didn't specify --debug.
>  return
>  
> -# TODO: add a "system defaults" config section so this default
> -# of more(1) can be easily replaced with a global
> -# configuration file. For example, on OS X the sane default is
> -# less(1), not more(1), and on debian it's
> -# sensible-pager(1). We should probably also give the system
> -# default editor command similar treatment.
> -envpager = encoding.environ.get('PAGER', 'more')
> +# sysdefault.pager is available for packagers or system
> +# administrators to specify a saner default pager for their
> +# environment.
> +defaultpager = self.config('sysdefault', 'pager', default='more')
> +envpager = encoding.environ.get('PAGER', defaultpager)
>  pagercmd = self.config('pager', 'pager', envpager)
>  if not pagercmd:
>  return
> @@ -1348,6 +1346,10 @@ class ui(object):
>  editor = 'E'
>  else:
>  editor = 'vi'
> +# sysdefault.editor is available for packagers or system
> +# administrators to specify a saner default editor for their
> +# environment.
> +editor = self.config("sysdefault", "editor", default=editor)
>  return (encoding.environ.get("HGEDITOR") or
>  self.config("ui", "editor") or
>  encoding.environ.get("VISUAL") or
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel