Submitter | timeless@mozdev.org |
---|---|
Date | Dec. 9, 2015, 6:31 p.m. |
Message ID | <4420c4fda75e7eb246cd.1449685885@waste.org> |
Download | mbox | patch |
Permalink | /patch/11951/ |
State | Deferred |
Headers | show |
Comments
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.
Patch
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")) # perhaps it's a path to a map or a template if ('/' in tmpl or '\\' in tmpl) and os.path.isfile(tmpl):