Re: [PATCH v5] diff: add config option relative

[Laurent Arnoud <laurent@spkdev.net>]

Hi Danh,

On Mon, May 18, 2020 at 08:56:56PM +0700, Đoàn Trần Công Danh wrote:
> Thanks for addressing my comment.
> 
> Sorry for detecting this late.
> Since I usually only look into end of mail-archive to see any
> interesting topic.
> 
> And sorry for this late email,
> I want to run full test before replying with this.
> 
> I've just seen this:
> https://lore.kernel.org/git/xmqq1rnk923o.fsf@gitster.c.googlers.com/
> 
> I've written some test and concluded that we'll need this fix-up
> to make sure git-format-patch(1) doesn't generate broken patch:

No worries this Junio is ok for this I've added a test instead in
t/t4014-format-patch.sh ensuring that format-patch respects the config
diff.relative on the v6 of the patch, let me know what you think

> Nitpick:
> 
> I don't think this option is too special to add a newline to separate
> it from the rest :)
> 
> Sorry about not seeing this earlier, I'm a very careless person.
> 
> Anyway, I think (just a matter of my _personal_ preference),
> it's better to move it up 21 lines, together with:
> 
> 	options->flags.rename_empty = 1;
> 

Applied in the v6

> > @@ -86,4 +87,80 @@ do
> >  	check_$type . dir/file2 --relative=sub
> >  done
> > 
> > +	diff --git a/$expect b/$expect
> > +	new file mode 100644
> > +	index 0000000..$short_blob_file2
> > +	--- /dev/null
> > +	+++ b/$expect
> > +	@@ -0,0 +1 @@
> > +	+other content
> > +	EOF
> > +	test_expect_success "config diff.relative $relative_opt -p $*" "
> > +		test_config -C $dir diff.relative $relative_opt &&
> > +		git -C '$dir' diff -p $* HEAD^ >actual &&
> > +		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
> 
> Please this leftover from debugging.

Opps fixed and thanks again for the review

-- 
Laurent