git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Johannes Sixt <j.sixt@viscovery.net>
Subject: Re: [PATCH 1/3] quote: let caller reset buffer for quote_path_relative()
Date: Wed, 10 Oct 2012 14:13:13 -0700	[thread overview]
Message-ID: <7vd30qav4m.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1349868894-3579-2-git-send-email-pclouds@gmail.com> ("Nguyễn	Thái Ngọc Duy"'s message of "Wed, 10 Oct 2012 18:34:52 +0700")

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> quote_path_relative() resetting output buffer is sometimes unnecessary
> as the buffer has never been used, and some other times makes it
> harder for the caller to use (see builtin/grep.c, the caller has to
> insert a string after quote_path_relative)
>
> Move the buffer reset back to call sites when necessary.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  The answer for Jeff's XXX in his patch, why strbuf_insert() instead
>  of just adding in advance.

This sounds a lot stronger than "let" to me.  All existing callers
that assumed that buf to be emptied by the function now has to clear
it.  "quote: stop resetting output buffer of quote_path_relative()"
may better describe what this really does.

How should this interact with the logic in the called function that
used to say "if we ended up returning an empty string because the
path is the same as the base, we should give ./ back", and what
should the return value of this function be?

To answer these questions, you must define the meaning of the string
in the output buffer that already exists when the function is
called.  If the caller did this:

	strbuf_addstr(&out, "The path relative to your HOME is: ");
        quote_path_relative(path, pathlen, &out, "/home/pclouds/");

then the answers are "We still need to add ./ but !out->len is no
longer a good test to decide" and "It should point at the first byte
of what we added, not out->buf".

But if the caller did this instead:

	srcdir = "/src/";
	strbuf_addstr(&dst, "/dst/");
        quote_path_relative(path, pathlen, &dst, srcdir);
        printf("cp '%s' '%s'\n", path, dst->buf);

then it is nonsensical to add "./" when out->len is not zero when
the function returns.

So what does it mean to have an existing string in the output buffer
when calling the function?  Is it supposed to be a path to a
directory, or just a general uninterpreted string (e.g. a message)?

  reply	other threads:[~2012-10-10 21:13 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-09  9:03 'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktree Johannes Sixt
2012-10-09  9:38 ` Nguyen Thai Ngoc Duy
2012-10-09 12:01   ` Nguyen Thai Ngoc Duy
2012-10-09 12:41     ` Jeff King
2012-10-09 18:59       ` Junio C Hamano
2012-10-10  5:17         ` Nguyen Thai Ngoc Duy
2012-10-10  5:33           ` Junio C Hamano
2012-10-10  5:45             ` Nguyen Thai Ngoc Duy
2012-10-10 11:34         ` Nguyễn Thái Ngọc Duy
2012-10-10 11:34           ` [PATCH 1/3] quote: let caller reset buffer for quote_path_relative() Nguyễn Thái Ngọc Duy
2012-10-10 21:13             ` Junio C Hamano [this message]
2012-10-11 13:04               ` Nguyen Thai Ngoc Duy
2012-10-11 16:42                 ` Junio C Hamano
2012-10-10 11:34           ` [PATCH 2/3] grep: pass true path name to grep machinery Nguyễn Thái Ngọc Duy
2012-10-10 11:34           ` [PATCH 3/3] grep: stop looking at random places for .gitattributes Nguyễn Thái Ngọc Duy
2012-10-10 11:51             ` Johannes Sixt
2012-10-10 12:03               ` Nguyen Thai Ngoc Duy
2012-10-10 12:12                 ` Johannes Sixt
2012-10-10 12:32                   ` Nguyen Thai Ngoc Duy
2012-10-10 12:43                     ` Johannes Sixt
2012-10-10 12:51                       ` Nguyen Thai Ngoc Duy
2012-10-10 19:44                   ` Junio C Hamano
2012-10-11  5:55                     ` Johannes Sixt
2012-10-11  7:04                       ` Michael Haggerty
2012-10-11  8:17                         ` Nguyen Thai Ngoc Duy
2012-10-10 13:59           ` [PATCH v2 0/2] Re: 'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktree Nguyễn Thái Ngọc Duy
2012-10-10 13:59             ` [PATCH v2 1/2] quote: let caller reset buffer for quote_path_relative() Nguyễn Thái Ngọc Duy
2012-10-10 13:59             ` [PATCH v2 2/2] grep: stop looking at random places for .gitattributes Nguyễn Thái Ngọc Duy
2012-10-10 14:21               ` Johannes Sixt
2012-10-10 19:56                 ` Junio C Hamano
2012-10-11  5:45                   ` Johannes Sixt
2012-10-11 15:51                     ` Junio C Hamano
2012-10-12  7:33                       ` Johannes Sixt
2012-10-14  4:29                         ` Junio C Hamano
2012-10-15  6:02                           ` Johannes Sixt
2012-10-15 16:54                             ` Junio C Hamano
2012-10-16  6:39                               ` Johannes Sixt
2012-10-17  7:05                                 ` Johannes Sixt
2012-10-17  7:33                                   ` Junio C Hamano
2012-10-11  1:49                 ` Nguyen Thai Ngoc Duy
2012-10-11  3:15                   ` Junio C Hamano
2012-10-12 10:49               ` [PATCH v3] " Nguyễn Thái Ngọc Duy
2012-10-12 16:47                 ` 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=7vd30qav4m.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j.sixt@viscovery.net \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /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).