git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Stefan Beller <sbeller@google.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH 3/9] diff: avoid generating unused hunk header lines
Date: Sat, 03 Nov 2018 09:32:02 +0900	[thread overview]
Message-ID: <xmqqr2g3ngql.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20181102201549.GA17228@sigill.intra.peff.net> (Jeff King's message of "Fri, 2 Nov 2018 16:15:49 -0400")

Jeff King <peff@peff.net> writes:

>> to have *some* names there, for the sake of a
>> simply described coding style without many exceptions
>> (especially those exceptions that rely on judgement).
>
> Fair enough.

For the record, there aren't "many" exceptions to the rule that was
suggested earlier: if you refer to parameters by name in the comment
to explain the interface, name them and use these names [*1*].

> Squashable patch is below; it goes on 34c829621e (diff: avoid generating
> unused hunk header lines, 2018-11-02).
>
> Junio, let me know if you'd prefer a re-send of the series, but it
> doesn't look necessary otherwise (so far).

Giving them proper names would serve as a readily usable sample for
those who write new hunk-line callbacks that do not ignore them, so
what you wrote is good.  Let me squash it in.

Thanks.  If this were to add bunch of "unused$n" names to the
parameters to this no-op function, only to "have *some* names
there", I would have said that it is not even worth the time to
discuss it, though.

> diff --git a/xdiff-interface.h b/xdiff-interface.h
> index 7b0ccbdd1d..8af245eed9 100644
> --- a/xdiff-interface.h
> +++ b/xdiff-interface.h
> @@ -39,7 +39,9 @@ extern int git_xmerge_style;
>   * Can be used as a no-op hunk_fn for xdi_diff_outf(), since a NULL
>   * one just sends the hunk line to the line_fn callback).
>   */
> -void discard_hunk_line(void *, long, long, long, long, const char *, long);
> +void discard_hunk_line(void *priv,
> +		       long ob, long on, long nb, long nn,
> +		       const char *func, long funclen);
>  
>  /*
>   * Compare the strings l1 with l2 which are of size s1 and s2 respectively.
>
> -Peff

[Footnote]

*1* You may say that the "if you refer to parameters by name" part
relies on judgment, but I think it is a good thing---it makes poor
judgment stand out.

When describing a function that takes two parameters of the same
type, for example, you could explain the interface as "take two ints
and return true if first int is smaller than the second int" in
order to leave the parameters unnamed, but if you gave such a
description, it is so obvious that you are _avoiding_ names.  Not
using names when you do not have to is good, but nobody with half an
intelligence would think it is good to give a bad description just
to avoid using names.

  reply	other threads:[~2018-11-03  0:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-02  6:31 [PATCH 0/9] saner xdiff hunk callbacks Jeff King
2018-11-02  6:35 ` [PATCH 1/9] xdiff: provide a separate emit callback for hunks Jeff King
2018-11-02  6:35 ` [PATCH 2/9] xdiff-interface: provide a separate consume " Jeff King
2018-11-02  6:36 ` [PATCH 3/9] diff: avoid generating unused hunk header lines Jeff King
2018-11-02 19:50   ` Stefan Beller
2018-11-02 20:15     ` Jeff King
2018-11-03  0:32       ` Junio C Hamano [this message]
2018-11-02  6:36 ` [PATCH 4/9] diff: discard hunk headers for patch-ids earlier Jeff King
2018-11-02  6:37 ` [PATCH 5/9] diff: use hunk callback for word-diff Jeff King
2018-11-02  6:38 ` [PATCH 6/9] combine-diff: use an xdiff hunk callback Jeff King
2018-11-02  6:39 ` [PATCH 7/9] diff: convert --check to use a " Jeff King
2018-11-02  6:39 ` [PATCH 8/9] range-diff: " Jeff King
2018-11-02  6:40 ` [PATCH 9/9] xdiff-interface: drop parse_hunk_header() Jeff King
2018-11-02 11:48 ` [PATCH 0/9] saner xdiff hunk callbacks 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

  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=xmqqr2g3ngql.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sbeller@google.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.
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).