From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Keller, Jacob E" <jacob.e.keller@intel.com>,
"git@vger.kernel.org" <git@vger.kernel.org>,
"sunshine@sunshineco.com" <sunshine@sunshineco.com>
Subject: Re: [PATCH 3/3] tag: support configuring --sort via .gitconfig
Date: Tue, 15 Jul 2014 19:38:36 -0400 [thread overview]
Message-ID: <20140715233836.GA5572@peff.net> (raw)
In-Reply-To: <xmqqfvi2wqvq.fsf@gitster.dls.corp.google.com>
On Tue, Jul 15, 2014 at 09:03:53AM -0700, Junio C Hamano wrote:
> > Do we want to go this way?
>
> I do not speak for Peff, but I personally think this is just a "fun"
> demonstration, nothing more, and my gut feeling is that it would
> make things unnecessary complex without much real gain to pursue it
> further.
Yeah, it is a little too complicated for what it is buying us here. If
we had a real error stack with allocated error types or something, then
callers could do more useful programmatic things with it. But:
1. We usually only care about system errors in such a case, and get by
with using errno.
2. I would not want to annotate all of the library-ish functions with
case-specific return types. That is a lot of work for little gain.
I think my favorite of the suggestions so far is basically the two-line:
error: sort specification is bad...
warning: ignoring invalid tag.sort
There are tons of places in git where we already do this kind of
"error chaining" over multiple lines (and multiple calls to error()),
and it doesn't require any new code or techniques.
But what is in v8 is not so bad from my cursory glance.
-Peff
PS I am traveling this week and will probably be a lot less responsive.
Please don't let me hold up your conversations.
next prev parent reply other threads:[~2014-07-15 23:39 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-11 22:55 [PATCH 1/3] tag: use skip_prefix instead of magic numbers Jacob Keller
2014-07-11 22:55 ` [PATCH 2/3] tag: fix --sort tests to use cat<<-\EOF format Jacob Keller
2014-07-11 22:55 ` [PATCH 3/3] tag: support configuring --sort via .gitconfig Jacob Keller
2014-07-13 3:29 ` Eric Sunshine
2014-07-13 17:10 ` Junio C Hamano
2014-07-13 17:33 ` Jeff King
2014-07-13 18:36 ` Jeff King
2014-07-14 17:17 ` Junio C Hamano
2014-07-15 14:52 ` Keller, Jacob E
2014-07-15 16:03 ` Junio C Hamano
2014-07-15 17:27 ` Keller, Jacob E
2014-07-15 18:17 ` Junio C Hamano
2014-07-15 18:31 ` Keller, Jacob E
2014-07-15 19:12 ` Junio C Hamano
2014-07-15 20:29 ` Keller, Jacob E
2014-07-15 21:31 ` Keller, Jacob E
2014-07-15 19:10 ` Junio C Hamano
2014-07-15 23:38 ` Jeff King [this message]
2014-07-12 1:02 ` [PATCH 1/3] tag: use skip_prefix instead of magic numbers Jeff King
-- strict thread matches above, loose matches on Subject: below --
2014-07-11 20:51 Jacob Keller
2014-07-11 20:51 ` [PATCH 3/3] tag: support configuring --sort via .gitconfig Jacob Keller
2014-07-11 20:54 ` Keller, Jacob E
2014-07-11 21:06 ` Jeff King
2014-07-11 21:08 ` Keller, Jacob E
2014-07-11 22:17 ` Junio C Hamano
2014-07-11 22:30 ` Keller, Jacob E
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140715233836.GA5572@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jacob.e.keller@intel.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).