git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Taylor Blau <me@ttaylorr.com>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Jeff King <peff@peff.net>,
	Jeff Hostetler <jeffhost@microsoft.com>,
	Elijah Newren <newren@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [RFC PATCH 00/21] C99: show meaningful <file>:<line> in trace2 via macros
Date: Tue, 16 Nov 2021 14:36:46 -0500	[thread overview]
Message-ID: <YZQITrlvO7eUZyDN@nand.local> (raw)
In-Reply-To: <211116.86czmzq5w2.gmgdl@evledraar.gmail.com>

On Tue, Nov 16, 2021 at 07:58:01PM +0100, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Nov 16 2021, Taylor Blau wrote:
> >> At the end of this series we expose a config variable to have
> >> usage/die/warning emit line numbers. I.e. going from:
> >>
> >>     $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
> >>     fatal: bad boolean config value 'y' for 'core.x'
> >>
> >> To:
> >>
> >>     $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
> >>     fatal: config.c:1241: bad boolean config value 'y' for 'core.x'
> >
> > Just picking on this output change in particular. I agree that this is
> > easier for folks hacking on Git to trace down errors. But I'm not sure
> > that I could say then same about users, who will likely treat this extra
> > output as noise.
> >
> > Now we may find it helpful if they include it in a bug report, but I
> > feel reasonably comfortable saying that the value there is pretty
> > marginal. I don't find it all that problematic to grep for a specific
> > error string, and usually find myself in the right place.
>
> I wouldn't suggest exposing this to users, except perhaps as part of
> some "how to submit a bugreport" instructions. It's thoroughly optional.
>
> I thought it was easy enough to do with the preceding steps since all
> the data is there, and would help my workflow a lot.
>
> If you've got the file/line number like that you can make it clickable
> in your terminal/compile mode, e.g. Emacs's M-x compile. Saves time over
> having to grep manually select the string, grep for it etc.
>
> Anyway, I can certainly live with peeling this patch off the end and
> just stopping at the trace2 data for now, if you/others feel strongly
> about it.

I don't feel strongly, and I was just noting that it seemed like users
would treat this extra information more often as noise than anything
else.

When you talk about making it optional, do you mean through
configuration / an environment variable, or by including / not including
the patch? In other words, the latter seems much more like us making a
decision on whether or not to include line numbers rather than
presenting a new option to users, though I may be misunderstanding.

> As noted in 02/21 we're hard depending on this particular C99 feature
> already fon a few releases now, the only change on that front in this
> series is to stop committing to maintaining the non-C99 codepaths.

> We've already had hard dependencies on various bits of C99 for years now
> without any trouble, and I wouldn't expect any problems on this front
> either.

Interesting; so this and others are likely part of MSVC's kind-of
support for C99 features? In other words, that MSVC supports some
features from C99 (and we are depending on a subset of those) but not
all features so that it could reasonably be called a spec-compliant
compiler for the C99 standard? If so, makes sense.

Thanks,
Taylor

  reply	other threads:[~2021-11-16 19:36 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 22:18 [RFC PATCH 00/21] C99: show meaningful <file>:<line> in trace2 via macros Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 01/21] git-compat-util.h: clarify GCC v.s. C99-specific in comment Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 02/21] C99 support: hard-depend on C99 variadic macros Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 03/21] usage.c: add a die_message() routine Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 04/21] usage.c API users: use die_message() where appropriate Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 05/21] usage.c + gc: add and use a die_message_errno() Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 06/21] config API: don't use vreportf(), make it static in usage.c Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 07/21] common-main.c: call exit(), don't return Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 08/21] usage.c: add a non-fatal bug() function to go with BUG() Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 09/21] parse-options.[ch] API: use bug() to improve error output Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 10/21] receive-pack: use bug() and BUG_if_bug() Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 11/21] cache-tree.c: " Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 12/21] pack-objects: use BUG(...) not die("BUG: ...") Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 13/21] strbuf.h: " Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 14/21] usage API: create a new usage.h, move API docs there Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 15/21] usage.[ch] API users: use report_fn, not hardcoded prototype Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 16/21] usage.[ch] API: rename "warn" vars functions to "warning" Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 17/21] usage.c: move usage routines around Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 18/21] usage.c: move rename variables in " Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 19/21] usage API: use C99 macros for {usage,usagef,die,error,warning,die}*() Ævar Arnfjörð Bjarmason
2021-12-27 19:32   ` Jeff Hostetler
2021-12-27 23:01     ` Ævar Arnfjörð Bjarmason
2021-12-28 16:32       ` Jeff Hostetler
2021-12-28 18:51         ` Elijah Newren
2021-12-28 23:48           ` Ævar Arnfjörð Bjarmason
2021-12-29  2:15             ` Elijah Newren
2021-12-28 23:42         ` Ævar Arnfjörð Bjarmason
2021-12-29 16:13         ` Jeff Hostetler
2021-11-15 22:18 ` [RFC PATCH 20/21] usage API: make the "{usage,fatal,error,warning,BUG}: " translatable Ævar Arnfjörð Bjarmason
2021-11-15 22:18 ` [RFC PATCH 21/21] usage API: add "core.usageAddSource" config to add <file>:<line> Ævar Arnfjörð Bjarmason
2021-11-16 18:43 ` [RFC PATCH 00/21] C99: show meaningful <file>:<line> in trace2 via macros Taylor Blau
2021-11-16 18:58   ` Ævar Arnfjörð Bjarmason
2021-11-16 19:36     ` Taylor Blau [this message]
2021-11-16 20:16       ` Ævar Arnfjörð Bjarmason

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YZQITrlvO7eUZyDN@nand.local \
    --to=me@ttaylorr.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).