public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Joe Perches <joe@perches•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] git-apply - Add --include=PATH
Date: Sat, 23 Aug 2008 17:54:22 -0700	[thread overview]
Message-ID: <7viqtrw7up.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 1219523869.18365.106.camel@localhost

Joe Perches <joe@perches•com> writes:

> Add similar capability to --exclude=
>
> Allows selection of files to patch from a
> large patchset.
>
> Signed-off-by: Joe Perches <joe@perches•com>

Thanks; I don't see anything fundamentally wrong with what this patch
tries to achieve.

> @@ -2996,10 +2996,16 @@ static struct excludes {
>  	const char *path;
>  } *excludes;
>  
> +static struct includes {
> +	struct includes *next;
> +	const char *path;
> +} *includes;

Now this is ugly.  You can just add a new variable "*includes" that is of
exactly the same type as existing "*excludes" without introducing a new
type.

You should then find it disturbing that the shared type is still called
"struct excludes" even though it is now used for things you would want to
include.  You are right.  You can then either rename it to a more neutral
name, or (even better) use an existing type, such as "string_list".

Which would mean that this patch should be done as two patches:

 [1/2] builtin-apply.c: Use "string_list" for "--excludes".
 [2/2] builtin-apply.c: Add "--includes" option.

The first will be a preparatory step that does not change any externally
visible behaviour.  The only thing it will do is to change the type of
"excludes" to "struct string_list", to update the option parser to use
string_list_append() to add to it, and to update the way "use_patch()" to
iterate over the items in the exclude list.

The second will add a new "includes" variable, and do the moral equivalent
of what your patch did.

The first patch most likely should introduce a new helper function:

  int string_list_has_match(struct string_list *s, const char *path);

so that the update to "use_patch()" that needs to be done in the second
patch can just pass a different string_list to implement the additional
check.

next prev parent reply	other threads:[~2008-08-24  0:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-23 20:37 [PATCH] git-apply - Add --include=PATH Joe Perches
2008-08-24  0:54 ` Junio C Hamano [this message]
2008-08-24 21:57   ` Joe Perches
2008-08-25  8:05     ` 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:
  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=7viqtrw7up.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=joe@perches$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox