Re: [PATCH 4/6] require_clean_work_tree: ensure that the index was read

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 30, 2016, 10:50 UTC
Message-ID: <alpine.DEB.2.20.1608301241590.129229@virtualbox>
In-Reply-To: <xmqqmvjv6uxn.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 29 Aug 2016, Junio C Hamano wrote:

Show 6 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > The function would otherwise pretend to work fine, but totally ignore
> > the working directory.
> 
> s/^T/Unless the caller has already read the index, t/;

Changed. (I also removed the "otherwise" which would sound funny otherwise.)

Show 14 quoted lines

> I am not sure if it should be left as the responsibility of the
> caller (i.e. check the_index.initialized to bark at a caller that
> forgets to read from an index) or it is OK to unconditionally read
> from $GIT_INDEX_FILE in a truly libified function.  A caller that
> wants to read from a custom (temporary) index file can choose to
> make sure it does read_inddex_from() from its custom file before
> calling this function, but if it forgets to do so, the penalty is
> severe than ordinary callers who would have read from the normal
> index file anyway.
> 
> Even though the word-lego issue would make this particular API not
> yet suitable, "who is responsible for reading the index" is an
> orthogonal issue that we can discuss even on this version, so I
> thought I'd bring it up.

It is orthogonal alright. So I won't act on it, but just add my thoughts:

We might want to consider thinking about introducing a more consistent internal API. This is even more orthogonal to the patch under discussion than just teaching require_clean_work_tree() about different index files, of course.

It might very well pay off in the future were we to design a more unified "look & feel" to the API e.g. by putting more restrictions on the order of parameters, required "int" return values for functions that may fail, a unified error handling that does not necessarily print to stderr.

Having said that, I do not have time to take lead on that, either. :-)

Ciao, Dscho