All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Stefan Beller <sbeller@google.com>,
	"git\@vger.kernel.org" <git@vger.kernel.org>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH 1/4] strbuf: migrate api-strbuf.txt documentation to strbuf.h
Date: Fri, 12 Dec 2014 14:16:32 -0800	[thread overview]
Message-ID: <xmqqoar8ecov.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20141212212800.GA27451@peff.net> (Jeff King's message of "Fri, 12 Dec 2014 16:28:00 -0500")

Jeff King <peff@peff.net> writes:

> +/**
> + * * Related to the contents of the buffer
> + */
> +
> +/**
> + * Strip whitespace from the beginning and end of a string.
> + * Equivalent to performing `strbuf_rtrim()` followed by `strbuf_ltrim()`.
> + */
>  extern void strbuf_trim(struct strbuf *);
> +
> +/**
> + * Strip whitespace from the end of a string.
> + */
>  extern void strbuf_rtrim(struct strbuf *);
> +
> +/**
> + * Strip whitespace from the beginning of a string.
> + */
>  extern void strbuf_ltrim(struct strbuf *);

This part reminds me why I often find the current "out of header"
documentation when writing code _without_ the need for _learning_
the API.  The corresponding part in the original is merely these:

    /*----- content related -----*/
    extern void strbuf_trim(struct strbuf *);
    extern void strbuf_rtrim(struct strbuf *);
    extern void strbuf_ltrim(struct strbuf *);
    extern int strbuf_reencode(struct strbuf *sb, const char *from, const char *to);
    ...

It is a lot quicker, scanning this with eyeball, to notice that
there are three related "trim" functions and how they are spelled.
If I wanted to "trim from both ends", knew there were some ways to
"trim" but didn't remember if we had r/ltrim without trim, and/or
was not sure if the "both" version was was _trim or _btrim, scanning
a bland list tends to be much more pleasant.

When you are _learning_ the API, the format that spreads each
individual function into its own section with explanation would be
easier to read through.

Not a strong objection to the overall direction, but I think that we
need to be aware that we are making things harder for some use
cases.

  parent reply	other threads:[~2014-12-12 22:16 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-06  1:51 [PATCH] document string_list_clear Stefan Beller
2014-12-06  2:04 ` Jonathan Nieder
2014-12-06  5:27   ` Jeff King
2014-12-06  5:30   ` Jeff King
2014-12-09 19:41     ` Junio C Hamano
2014-12-09 20:15       ` Jeff King
2014-12-09 20:21         ` Jonathan Nieder
2014-12-09 19:37   ` Junio C Hamano
2014-12-09 19:48     ` Stefan Beller
2014-12-09 20:17       ` Jonathan Nieder
2014-12-09 20:27         ` Jeff King
2014-12-09 20:32           ` Stefan Beller
2014-12-09 20:46             ` Jeff King
2014-12-09 22:23           ` Jonathan Nieder
2014-12-09 23:18             ` Junio C Hamano
2014-12-10  8:52               ` Jeff King
2014-12-10  8:43             ` Jeff King
2014-12-10  9:18               ` Jonathan Nieder
2014-12-12  9:16                 ` Jeff King
2014-12-12 18:31                   ` Jonathan Nieder
2014-12-12 19:17                     ` Junio C Hamano
2014-12-12 19:19                     ` Stefan Beller
2014-12-12 19:29                       ` Jeff King
2014-12-12 19:24                     ` Jeff King
2014-12-12 19:35                       ` Jonathan Nieder
2014-12-12 21:27                         ` Jeff King
2014-12-12 21:28                           ` [PATCH 1/4] strbuf: migrate api-strbuf.txt documentation to strbuf.h Jeff King
2014-12-12 21:40                             ` Jeff King
2014-12-12 22:16                             ` Junio C Hamano [this message]
2014-12-12 22:30                             ` Jonathan Nieder
2014-12-12 21:28                           ` [PATCH 2/4] strbuf.h: drop asciidoc list formatting from API docs Jeff King
2014-12-12 22:19                             ` Junio C Hamano
2014-12-12 22:37                             ` Jonathan Nieder
2014-12-12 21:30                           ` [PATCH 3/4] strbuf.h: format asciidoc code blocks as 4-space indent Jeff King
2014-12-12 22:39                             ` Jonathan Nieder
2014-12-14 17:42                               ` Michael Haggerty
2014-12-12 21:32                           ` [PATCH 4/4] strbuf.h: reorganize api function grouping headers Jeff King
2014-12-12 22:46                             ` Jonathan Nieder
2014-12-12 22:32                           ` [PATCH] document string_list_clear Stefan Beller
2014-12-10 20:09               ` Michael Haggerty
2014-12-10 21:51                 ` Jonathan Nieder
2014-12-10 22:28                   ` Junio C Hamano
2014-12-10 22:37                     ` Jonathan Nieder
2014-12-10 23:04                       ` Junio C Hamano
2014-12-10 23:08                         ` Jonathan Nieder
2014-12-09 22:49         ` Jonathan Nieder
2014-12-09 23:07           ` Stefan Beller
2014-12-09 23:15             ` Jonathan Nieder
2014-12-09 20:00     ` Jonathan Nieder

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=xmqqoar8ecov.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.