Re: [PATCH] Teach git-diff{,-files} the new option `--no-index`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 22, 2007, 20:01 UTC
Message-ID: <Pine.LNX.4.63.0702222051490.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vmz36f1bv.fsf@assigned-by-dhcp.cox.net>

Hi,

On Thu, 22 Feb 2007, Junio C Hamano wrote:

Show 12 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Can't they made to call a new function, which has the option
> >> parsing followed by the switch between magic max_count==-2
> >> codepath and run_diff_files()?
> >
> > How about adding "int always_use_index" to the signature of 
> > run_diff_files()?
> 
> The "magic" part is not about the index at all (it is about
> using only that magic part to do diff on filesystem), and the
> original code is about "index vs filesystem".

I don't understand. diff-files until --no-index _always_ diffed against the index. Now, if "allow_no_index" is non-zero, under very special circumstances diff-files does the DWIMery.

Show 28 quoted lines

> I would rather keep them in separate function.  That is:
> 
> 	run_diff_files(); /* compares index vs filesystem */
>                 
> 	run_diff_filesystem(); /* runs diff2 */
> 
>         run_diff_files_cmd() {
>              	/* a new helper */
> 		parse args;
> 		if (magic)
>                 	run_diff_filesystem();
> 		else
>                 	run_diff_files();
> 	}
> 
> 	builtin_diff() {
>         	if (not about two trees nor two blobs ...)
>                 	run_diff_files_cmd();
> 		else if (about two trees)
>                 	run_diff_tree();
> 		else if (about tree and index)
>                 	run_diff_index();
> 	}
> 
>         wt_status() {
>         	run_diff_files();
>                 run_diff_index();
> 	}

So you mean only add run_diff_files_cmd(), called by diff and diff-files?

> For one thing, I want to move read_cache() out of run_diff_files() and 
> run_diff_index() to their callers, so that the caller can use a 
> handcrafted active_cache[] that is somewhat different from what 
> read_cache() originally read.

I don't understand. Would it not be easier to "return active_nr" in read_cache_from(path) _also_ when active_cache is set? Or alternatively check if active_alloc is set?

Ciao, Dscho