Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH] strbuf_stripspace(): rename skip_comments arg to strip_comments
@ 2023-03-23 16:22 Oswald Buddenhagen
  2023-04-27  7:49 ` Oswald Buddenhagen
  0 siblings, 1 reply; 4+ messages in thread
From: Oswald Buddenhagen @ 2023-03-23 16:22 UTC (permalink / raw)
  To: git

That makes its function much clearer and more consistent with the
context.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 strbuf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index 1c57ac6574..49e8beaa4c 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1118,10 +1118,10 @@ static size_t cleanup(char *line, size_t len)
  *
  * If last line does not have a newline at the end, one is added.
  *
- * Enable skip_comments to skip every line starting with comment
+ * Enable strip_comments to strip every line starting with a comment
  * character.
  */
-void strbuf_stripspace(struct strbuf *sb, int skip_comments)
+void strbuf_stripspace(struct strbuf *sb, int strip_comments)
 {
 	size_t empties = 0;
 	size_t i, j, len, newlen;
@@ -1134,7 +1134,7 @@ void strbuf_stripspace(struct strbuf *sb, int skip_comments)
 		eol = memchr(sb->buf + i, '\n', sb->len - i);
 		len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
 
-		if (skip_comments && len && sb->buf[i] == comment_line_char) {
+		if (strip_comments && len && sb->buf[i] == comment_line_char) {
 			newlen = 0;
 			continue;
 		}
-- 
2.40.0.152.g15d061e6df


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] strbuf_stripspace(): rename skip_comments arg to strip_comments
  2023-03-23 16:22 [PATCH] strbuf_stripspace(): rename skip_comments arg to strip_comments Oswald Buddenhagen
@ 2023-04-27  7:49 ` Oswald Buddenhagen
  2023-04-27  8:32   ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: Oswald Buddenhagen @ 2023-04-27  7:49 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano

ping!

On Thu, Mar 23, 2023 at 05:22:34PM +0100, Oswald Buddenhagen wrote:
>That makes its function much clearer and more consistent with the
>context.
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] strbuf_stripspace(): rename skip_comments arg to strip_comments
  2023-04-27  7:49 ` Oswald Buddenhagen
@ 2023-04-27  8:32   ` Jeff King
  2023-05-02 19:00     ` Felipe Contreras
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2023-04-27  8:32 UTC (permalink / raw)
  To: Oswald Buddenhagen; +Cc: git, Junio C Hamano

On Thu, Apr 27, 2023 at 09:49:48AM +0200, Oswald Buddenhagen wrote:

> ping!
> 
> On Thu, Mar 23, 2023 at 05:22:34PM +0100, Oswald Buddenhagen wrote:
> > That makes its function much clearer and more consistent with the
> > context.

A quick aside on workflow:

  - It's customary on this list to just re-send the patch (with a note
    that it's a re-send below the "---"), rather than a "ping" email.
    They both potentially get more people's attention, but one doesn't
    require people to dig up the old thread. :)

  - We usually reserve cc-ing for people who have touched that area, or
    participated in previous discussions on the topic. That helps people
    manage the list traffic by seeing which threads somebody else
    thought would be important for them to see. But here I think I'm on
    the cc because I'm a sucker who will review anything that gets cc'd
    to me. A better cc might be folks who added the parameter, though it
    has been long enough that they may or may not be active.

I'll quote liberally from the original below:

On Thu, Mar 23, 2023 at 05:22:34PM +0100, Oswald Buddenhagen wrote:

> That makes its function much clearer and more consistent with the
> context.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---
>  strbuf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/strbuf.c b/strbuf.c
> index 1c57ac6574..49e8beaa4c 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -1118,10 +1118,10 @@ static size_t cleanup(char *line, size_t len)
>   *
>   * If last line does not have a newline at the end, one is added.
>   *
> - * Enable skip_comments to skip every line starting with comment
> + * Enable strip_comments to strip every line starting with a comment
>   * character.
>   */
> -void strbuf_stripspace(struct strbuf *sb, int skip_comments)
> +void strbuf_stripspace(struct strbuf *sb, int strip_comments)

I don't have a strong opinion either way. The original name was just
copied from stripspace()'s similar parameter, which came from 9690c118fa
(Fix git-stripspace to process correctly long lines and spaces.,
2007-06-25). I don't think it carries any particular significance,
though I find either name easy enough to understand.

-Peff

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] strbuf_stripspace(): rename skip_comments arg to strip_comments
  2023-04-27  8:32   ` Jeff King
@ 2023-05-02 19:00     ` Felipe Contreras
  0 siblings, 0 replies; 4+ messages in thread
From: Felipe Contreras @ 2023-05-02 19:00 UTC (permalink / raw)
  To: Jeff King, Oswald Buddenhagen; +Cc: git, Junio C Hamano

Jeff King wrote:
> I'll quote liberally from the original below:
> 
> On Thu, Mar 23, 2023 at 05:22:34PM +0100, Oswald Buddenhagen wrote:
> 
> > That makes its function much clearer and more consistent with the
> > context.
> > 
> > Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> > ---
> >  strbuf.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/strbuf.c b/strbuf.c
> > index 1c57ac6574..49e8beaa4c 100644
> > --- a/strbuf.c
> > +++ b/strbuf.c
> > @@ -1118,10 +1118,10 @@ static size_t cleanup(char *line, size_t len)
> >   *
> >   * If last line does not have a newline at the end, one is added.
> >   *
> > - * Enable skip_comments to skip every line starting with comment
> > + * Enable strip_comments to strip every line starting with a comment
> >   * character.
> >   */
> > -void strbuf_stripspace(struct strbuf *sb, int skip_comments)
> > +void strbuf_stripspace(struct strbuf *sb, int strip_comments)
> 
> I don't have a strong opinion either way. The original name was just
> copied from stripspace()'s similar parameter, which came from 9690c118fa
> (Fix git-stripspace to process correctly long lines and spaces.,
> 2007-06-25). I don't think it carries any particular significance,
> though I find either name easy enough to understand.

To me strip_comments makes more sense, because it implies an active
action, skip_comments implies nothing will be done on them.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-05-02 19:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-23 16:22 [PATCH] strbuf_stripspace(): rename skip_comments arg to strip_comments Oswald Buddenhagen
2023-04-27  7:49 ` Oswald Buddenhagen
2023-04-27  8:32   ` Jeff King
2023-05-02 19:00     ` Felipe Contreras

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