Patchwork [2,of,2] formatter: lookuptemplate make -Tlist exit without an error

# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1449685780 0
#      Wed Dec 09 18:29:40 2015 +0000
# Node ID 4420c4fda75e7eb246cd6ed04620f61507bd7f45
# Parent  2f84f71b9b546f121e9f94771e268b3b0cef0956
formatter: lookuptemplate make -Tlist exit without an error

On Wed, 09 Dec 2015 12:31:25 -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1449685780 0
> #      Wed Dec 09 18:29:40 2015 +0000
> # Node ID 4420c4fda75e7eb246cd6ed04620f61507bd7f45
> # Parent  2f84f71b9b546f121e9f94771e268b3b0cef0956
> formatter: lookuptemplate make -Tlist exit without an error
> 
> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
> --- a/mercurial/formatter.py
> +++ b/mercurial/formatter.py
> @@ -179,7 +179,7 @@
>  
>      if tmpl == 'list':
>          ui.write(_("available styles: %s\n") % templater.stylelist())
> -        raise error.Abort(_("specify a template"))
> +        raise error.EarlyNormalExit(_("templates listed"))

I think it's acceptable to exit with an error because "log -Tlist" is
equivalent to the deprecated "log --style nosuch" option. And the log command
doesn't finish its job.

If those two are equivalent, we should split them.

If I'm asking `log -T list`, I'm asking for a list of styles, not a log.

If I ask for `log -T nosuch`, and nosuch doesn't exist, then that's an error.

I'm happy to write the code to split these cases.

On Thu, Dec 10, 2015 at 10:49 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Wed, 09 Dec 2015 12:31:25 -0600, timeless wrote:
>> # HG changeset patch
>> # User timeless <timeless@mozdev.org>
>> # Date 1449685780 0
>> #      Wed Dec 09 18:29:40 2015 +0000
>> # Node ID 4420c4fda75e7eb246cd6ed04620f61507bd7f45
>> # Parent  2f84f71b9b546f121e9f94771e268b3b0cef0956
>> formatter: lookuptemplate make -Tlist exit without an error
>>
>> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
>> --- a/mercurial/formatter.py
>> +++ b/mercurial/formatter.py
>> @@ -179,7 +179,7 @@
>>
>>      if tmpl == 'list':
>>          ui.write(_("available styles: %s\n") % templater.stylelist())
>> -        raise error.Abort(_("specify a template"))
>> +        raise error.EarlyNormalExit(_("templates listed"))
>
> I think it's acceptable to exit with an error because "log -Tlist" is
> equivalent to the deprecated "log --style nosuch" option. And the log command
> doesn't finish its job.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

On 12/09/2015 06:31 PM, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1449685780 0
> #      Wed Dec 09 18:29:40 2015 +0000
> # Node ID 4420c4fda75e7eb246cd6ed04620f61507bd7f45
> # Parent  2f84f71b9b546f121e9f94771e268b3b0cef0956
> formatter: lookuptemplate make -Tlist exit without an error
>
> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
> --- a/mercurial/formatter.py
> +++ b/mercurial/formatter.py
> @@ -179,7 +179,7 @@
>
>       if tmpl == 'list':
>           ui.write(_("available styles: %s\n") % templater.stylelist())
> -        raise error.Abort(_("specify a template"))
> +        raise error.EarlyNormalExit(_("templates listed"))

using exception in random code to denote normal execution flow scares 
me. Given that yuya have other feedback I'm dropping this series from 
patchwork.


>
>       # perhaps it's a path to a map or a template
>       if ('/' in tmpl or '\\' in tmpl) and os.path.isfile(tmpl):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>

On Thu, 10 Dec 2015 12:46:00 -0500, timeless wrote:
> If those two are equivalent, we should split them.
> 
> If I'm asking `log -T list`, I'm asking for a list of styles, not a log.
> 
> If I ask for `log -T nosuch`, and nosuch doesn't exist, then that's an error.

It sounds strange to me that "log -T $valid_template" shows log, but
"log -T list" doesn't and both exit with 0. So I consider "list" as the
special style name to error out.

> If I ask for `log -T nosuch`, and nosuch doesn't exist, then that's an error.
> I'm happy to write the code to split these cases.
So, this isn't what happens -- at all.
First of all, they're already split up. Second, nosuch is not an
error, and the reason is that:

-T and --style are not equivalent.

Here's what happens for both (with the above patch applied):

[timeless@gcc2-power8 crew]$ hg log --style foeey; echo $?; hg log
--style list; echo $?; hg log -Tlist; echo $?; hg log -Tlisto -r .;
echo $?
abort: style 'foeey' not found
(available styles: bisect, changelog, compact, default, phases, status, xml)
255
abort: style 'list' not found
(available styles: bisect, changelog, compact, default, phases, status, xml)
255
available styles: bisect, changelog, compact, default, phases, status, xml
0
listo
0

Specifically, -T takes strings and will use them as templates, and it
will succeed (return 0)

Augie was ok w/ me making -Tlist not be an error, and I'm still
confident it's the right behavior given the intent of -Tlist.

On Fri, Dec 11, 2015 at 8:30 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Thu, 10 Dec 2015 12:46:00 -0500, timeless wrote:
>> If those two are equivalent, we should split them.
>>
>> If I'm asking `log -T list`, I'm asking for a list of styles, not a log.
>>
>> If I ask for `log -T nosuch`, and nosuch doesn't exist, then that's an error.
>
> It sounds strange to me that "log -T $valid_template" shows log, but
> "log -T list" doesn't and both exit with 0. So I consider "list" as the
> special style name to error out.

On Tue, 15 Dec 2015 12:21:13 -0500, timeless wrote:
> > If I ask for `log -T nosuch`, and nosuch doesn't exist, then that's an error.
> > I'm happy to write the code to split these cases.
> So, this isn't what happens -- at all.
> First of all, they're already split up. Second, nosuch is not an
> error, and the reason is that:
> 
> -T and --style are not equivalent.

I meant "-T list" is equivalent to "--style nosuch". It was introduced
at 0483ff40e326 because "--style nosuch" could list styles but --style
was deprecated.

> Specifically, -T takes strings and will use them as templates, and it
> will succeed (return 0)

Yes, that's the reason we have to handle "-T list" exceptionally.

> Augie was ok w/ me making -Tlist not be an error, and I'm still
> confident it's the right behavior given the intent of -Tlist.

Well, I won't stick to my opinion. But if -Tlist is not an error, raising
exception doesn't make sense at all. Then, all templatable commands would
have to handle -Tlist specially (or put some hack in dispatcher).

I think this is because -Tlist was designed as a replacement for --style nosuch.
We generally provide this sort of information by "hg help", but not for the
list of styles.