Re: [PATCH 2/4] diff.c: return filepair from diff_unmerge()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Some unrelated style comments below.

On Sun, Apr 24, 2011 at 5:51 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> The underlying diff_queue() returns diff_filepair so that the caller can
> further add information to it, and the helper function diff_unmerge()
> utilizes the feature itself, but does not expose it to its callers, which
> was kind of selfish.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> Âdiff.c | Â 13 ++++++++-----
> Âdiff.h | Â Â2 +-
> Â2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 9a5c77c..4c34c64 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4308,20 +4308,23 @@ void diff_change(struct diff_options *options,
> Â Â Â Â Â Â Â ÂDIFF_OPT_SET(options, HAS_CHANGES);
> Â}
>
> -void diff_unmerge(struct diff_options *options,
> - Â Â Â Â Â Â Â Â const char *path,
> - Â Â Â Â Â Â Â Â unsigned mode, const unsigned char *sha1)
> +struct diff_filepair *diff_unmerge(struct diff_options *options,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char *path,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned mode, const unsigned char *sha1)
> Â{

While you are here, why not write one arg per line?

> + Â Â Â struct diff_filepair *pair;
> Â Â Â Âstruct diff_filespec *one, *two;
>
> Â Â Â Âif (options->prefix &&
> Â Â Â Â Â Âstrncmp(path, options->prefix, options->prefix_length))
> - Â Â Â Â Â Â Â return;
> + Â Â Â Â Â Â Â return NULL;
>
> Â Â Â Âone = alloc_filespec(path);
> Â Â Â Âtwo = alloc_filespec(path);
> Â Â Â Âfill_filespec(one, sha1, mode);
> - Â Â Â diff_queue(&diff_queued_diff, one, two)->is_unmerged = 1;
> + Â Â Â pair = diff_queue(&diff_queued_diff, one, two);
> + Â Â Â pair->is_unmerged = 1;
> + Â Â Â return pair;
> Â}
>
> Âstatic char *run_textconv(const char *pgm, struct diff_filespec *spec,
> diff --git a/diff.h b/diff.h
> index bf2f44d..f51a8ee 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -209,7 +209,7 @@ extern void diff_change(struct diff_options *,
> Â Â Â Â Â Â Â Â Â Â Â Âconst char *fullpath,
> Â Â Â Â Â Â Â Â Â Â Â Âunsigned dirty_submodule1, unsigned dirty_submodule2);
>
> -extern void diff_unmerge(struct diff_options *,
> +extern struct diff_filepair *diff_unmerge(struct diff_options *,

While you are here, why not add the argument name |options| here too?

> Â Â Â Â Â Â Â Â Â Â Â Â const char *path,
> Â Â Â Â Â Â Â Â Â Â Â Â unsigned mode,
> Â Â Â Â Â Â Â Â Â Â Â Â const unsigned char *sha1);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]