From: Junio C Hamano <gitster@pobox•com>
To: "René Scharfe" <l.s.r@web•de>
Cc: Eric Sunshine <sunshine@sunshineco•com>,
Git List <git@vger•kernel.org>,
Vegard Nossum <vegard.nossum@oracle•com>
Subject: Re: [PATCH 1/6] t4051: add test for comments preceding function lines
Date: Mon, 20 Nov 2017 09:36:57 +0900 [thread overview]
Message-ID: <xmqqine5okeu.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <bde81d14-a955-4ea4-5799-2a95511d8215@web.de> ("René Scharfe"'s message of "Sun, 19 Nov 2017 11:02:49 +0100")
René Scharfe <l.s.r@web•de> writes:
> your suggested full-comment metric, i.e. more than nothing. But more
> importantly it's the actual comment payload. The leading "/*" line is
> included as a consequence of the employed heuristic, but a more
> refined one might omit it as it doesn't actually contain any comment.
I am slightly in favor of than against the above reasoning, but it
probably deserves to be recorded somewhere more readily accessible
than the mailing list archive. The title of the test "context
*includes* comment" can be read to hint it by not saying that the
precontext shows the *entire* comment, but that is a very weak hint
that will be missed by anybody unaware of the reasoning behind this
decision.
When showing function context it would be helpful to show comments
immediately before declarations, as they are most likely relevant. Add
a test for that.
... but without specifying the choice of lines too rigidly in the
test---we may want to stop before and not include "/*" in the
future, for example.
perhaps?
next prev parent reply other threads:[~2017-11-20 0:39 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-18 18:01 [PATCH 0/6] show non-empty lines before functions with diff/grep -W René Scharfe
2017-11-18 18:04 ` [PATCH 1/6] t4051: add test for comments preceding function lines René Scharfe
2017-11-19 1:18 ` Eric Sunshine
2017-11-19 10:02 ` René Scharfe
2017-11-20 0:36 ` Junio C Hamano [this message]
2017-11-20 17:28 ` René Scharfe
2017-11-21 0:37 ` Junio C Hamano
2017-11-18 18:04 ` [PATCH 2/6] xdiff: factor out is_func_rec() René Scharfe
2017-11-18 18:05 ` [PATCH 3/6] xdiff: show non-empty lines before functions with -W René Scharfe
2017-11-18 18:06 ` [PATCH 4/6] t7810: improve check of -W with user-defined function lines René Scharfe
2017-11-18 18:07 ` [PATCH 5/6] grep: update boundary variable for pre-context René Scharfe
2017-11-18 18:08 ` [PATCH 6/6] grep: show non-empty lines before functions with -W René Scharfe
2017-11-20 20:39 ` Stefan Beller
2017-11-20 22:07 ` René Scharfe
2017-11-21 23:35 ` Stefan Beller
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=xmqqine5okeu.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=l.s.r@web$(echo .)de \
--cc=sunshine@sunshineco$(echo .)com \
--cc=vegard.nossum@oracle$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox