Re: [PATCH v4 2/3] pretty: enable --expand-tabs by default for selected pretty formats

Jeff King [off-list ref] writes:

On Tue, Mar 29, 2016 at 04:15:08PM -0700, Junio C Hamano wrote:

quoted

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4fb5c76..23967b6 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt

@@ -43,10 +43,16 @@ people using 80-column terminals.
 	commit may be copied to the output.
 
 --expand-tabs::
+--no-expand-tabs::
 	Perform a tab expansion (replace each tab with enough number
 	of spaces to fill to the next display column that is
 	multiple of 8) in the log message before using the message
 	to show in the output.
++
+By default, tabs are expanded in pretty formats that indent the log
+message by 4 spaces (i.e.  'medium', which is the default, 'full',
+and "fuller').  `--no-expand-tabs` option can be used to disable
+this.

Mismatched quote types on "fuller".

Thanks.

quoted

@@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 
 	rev->commit_format = commit_format->format;
 	rev->use_terminator = commit_format->is_tformat;
+	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
 	if (commit_format->format == CMIT_FMT_USERFORMAT) {
 		save_user_format(rev, commit_format->user_format,
 				 commit_format->is_tformat);

This feels like the wrong time to set the value in rev_info, as it means
that:

  git log --no-expand-tabs --pretty=full

and

  git log --pretty=full --no-expand-tabs

behave differently.

I was sort of hoping that we can get away by defining that "an
explicit --pretty asks for the full behaviour of the format it
specifies, e.g. if you ask --pretty=medium, you are asking for
4-space indented tab-expanded log with the headers at the medium
level of detail".

The other values set in get_commit_format, like "use_terminator",
are inherently part of the format, but I don't think this is.

IOW, I was hoping nobody would agree with that and rather everybody
would consider tab-expansion is part of the format.

Let me try your way instead and report how it went when I send out a
reroll.

Likewise, if we were to eventually add config like "[log]expandtab = 4",
it should not be overridden by "--pretty=full" (but we probably _would_
want to have it kick in only for certain formats).

This is exactly why I didn't do a configuration variable, as I think
we can make only 50% of people happy.  Some would say "when I
explicitly ask for the "email" format, I expect that expandtab
configuration gets ignored" while others would say "I said I want
expandtab in the configuration no matter what".