All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Tanay Abhra <tanayabh@gmail.com>,
	git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>
Subject: Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API
Date: Thu, 07 Aug 2014 13:31:13 -0700	[thread overview]
Message-ID: <xmqqoavwjb3i.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <vpq61i4nld7.fsf@anie.imag.fr> (Matthieu Moy's message of "Thu, 07 Aug 2014 21:35:48 +0200")

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>>> Why is this needed? Are you now using key_value_info outside config.c?
>>> Or is it a leftover from a previous experiment?
>>
>> Has this been resolved in the new round?
>
> Tanay explained in another subthread why this was needed. For callers
> iterating over the string_list who want to get the file/line info, they
> need to be able to cast the void * pointer to struct key_value_info *.

For callers that want to see all the multi-values, it would be
preferrable for the iterator to pass the filename and the linenumber
to the callback function, instead of exposing its implementation
detail as a single string list and telling them to pick it apart,
no?

Not a very convincing argument, but OK for now in the sense that we
can fix it later if we wanted to before it gets too late.

  reply	other threads:[~2014-08-07 20:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-06 14:53 [PATCH v8 0/8] Rewrite `git_config()` using config-set API Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 1/8] config.c: mark error and warnings strings for translation Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 2/8] config.c: fix accuracy of line number in errors Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 3/8] add line number and file name info to `config_set` Tanay Abhra
2014-08-06 15:49   ` Ramsay Jones
2014-08-06 14:53 ` [PATCH v8 4/8] change `git_config()` return value to void Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 5/8] config: add `git_die_config()` to the config-set API Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 6/8] rewrite git_config() to use " Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 7/8] add a test for semantic errors in config files Tanay Abhra
2014-08-06 14:53 ` [PATCH v8 8/8] add tests for `git_config_get_string_const()` Tanay Abhra
2014-08-06 15:32   ` Matthieu Moy
2014-08-06 15:44     ` Tanay Abhra
2014-08-06 21:22     ` Junio C Hamano
2014-08-07  8:30       ` Matthieu Moy
2014-08-06 15:26 ` [PATCH v8 0/8] Rewrite `git_config()` using config-set API Matthieu Moy
2014-08-07 19:03   ` Junio C Hamano
2014-08-07 19:35     ` Matthieu Moy
2014-08-07 20:31       ` Junio C Hamano [this message]
2014-08-08 14:07         ` Tanay Abhra
2014-08-08 14:32           ` Ramsay Jones
2014-08-10 17:29             ` Junio C Hamano
2014-08-11  9:59               ` Ramsay Jones

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=xmqqoavwjb3i.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=tanayabh@gmail.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.