mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <>
To: Emily Shaffer <>
Subject: Re: [RFC PATCH] grep: provide sane default to grep_source struct
Date: Thu, 16 May 2019 12:07:12 +0900	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Emily Shaffer's message of "Wed, 15 May 2019 19:00:23 -0700")

Emily Shaffer <> writes:

> I don't know if adding a placeholder name is the right answer (hence RFC
> patch).
> Jonathan Nieder proposed alternatively adding some check to grep_source()
> to ensure that if opt->status_only is unset, gs->name must be non-NULL
> (and yell about it if not), as well as some extra comments indicating
> what assumptions are made about the data coming into functions like
> grep_source(). I'm fine with that as well (although I'm not sure it

That's an interesting problem.

Currently the only use of the function is to see if the log message
matches with the given pattern (yes/no), but it is conceivable that
new callers may want to prepare in-core data and use it to see if
that matches the pattern, or even to _show_ the lines that match the
pattern (possibly with its own .output callback).  Such a caller may
even make repeated calls to the function, and want to give hint in
the output to help users identify which in-core data produced hits,
which implies that a hardcoded "(in core)" is not such a great idea.
In a future where we support such a caller, the function signature
of grep_buffer() would change to include the name, and you would be
passing that to grep_source_init().

Until that happens, it would be OK to use a hardcoded "in-core"
constant here, but to futureproof for such a future, I think it
makes sense to have a check for a BUG() suggested by jrn---that
would become useful once callers of grep_buffer() can give names
prevent them from passing NULL when it later would be used.

> makes sense semantically to require a name which the user probably can't
> easily set, or else ban the user from printing LOC during grep). Mostly
> I'm happy with any solution besides a segfault with no error logging :)
> Thanks in advance for everyone's thoughts.
> Emily
>  grep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/grep.c b/grep.c
> index 0d50598acd..fd84454faf 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -2045,7 +2045,7 @@ int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
>  	struct grep_source gs;
>  	int r;
> -	grep_source_init(&gs, GREP_SOURCE_BUF, NULL, NULL, NULL);
> +	grep_source_init(&gs, GREP_SOURCE_BUF, _("(in memory)"), NULL, NULL);
>  	gs.buf = buf;
>  	gs.size = size;

  reply	other threads:[~2019-05-16  3:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16  2:00 [RFC PATCH] grep: provide sane default to grep_source struct Emily Shaffer
2019-05-16  3:07 ` Junio C Hamano [this message]
2019-05-16  3:13   ` Jonathan Nieder
2019-05-16  3:11 ` Jonathan Nieder
2019-05-16 21:05   ` Emily Shaffer
2019-05-16 21:44 ` [PATCH v2] " Emily Shaffer
2019-05-16 22:02   ` Jeff King
2019-05-21 23:52     ` Emily Shaffer
2019-05-22  0:17       ` Jonathan Nieder
2019-05-22  0:34   ` [PATCH v3] " Emily Shaffer
2019-05-22  0:58     ` Jonathan Nieder
2019-05-22  4:01     ` Jeff King
2019-05-23 20:23     ` [PATCH v4] grep: fail if call could output and name is null Emily Shaffer
2019-05-23 20:34       ` Jonathan Nieder
2019-05-28 17:57         ` Junio C Hamano

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:

  List information:

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

  git send-email \ \ \ \ \ \

* 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

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).