Re: [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2016, 21:52 UTC
Message-ID: <xmqqoaag9177.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1603142151230.4690@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Also, you might want to join my discussion with Junio about the sense or
> nonsense of keeping the prefix parameter instead of silently removing it
> while moving the functions.

This is a tangent to Paul's topic, but it is an important tangent in the other thread, in that you didn't mention one thing there that I needed to make an accurate assessment. I wasn't aware of your plan of moving it and use it in a context unrelated to "git pull", hence keeping the prefix would made perfect sense, as the enhanced error reporting (i.e. "not only I am saying that you have modified files and hence you cannot proceed, I can tell you which paths have been modified") would happen inside the function if done in that context.

If the function will be made a public helper that may be called by anybody, a possible error reporting mechanism would be to give a list of modified paths to the caller that asks them, and have the caller apply its own "prefix" processing to make them relative. The public helper function will not even be a position to say "you have modified files and hence you cannot proceed"--it will not be in a position to even issue an error message. The only thing it should do is to communicate to the caller if there are modified files or not (and leaving the decision on what to do to the caller--after all, the caller may want to do something only when there are modified files, e.g. "add ." may decide not to do anything when there is no change--so "hence you cannot proceed" is not its business), and if the caller wants to see them, which paths are dirty.

Incidentally, that is how wt_shortstatus_status() reports the list of modified paths, using s->prefix.