git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Add log.abbrev-commit config option

From
Jonathan Nieder <jrnieder@gmail.com>
Date
May 14, 2011, 19:01 UTC
Message-ID
<20110514190122.GA16851@elie>
In-Reply-To
<1305393758-95432-1-git-send-email-jaysoffian@gmail.com>
Hi,
I like the idea.
Jay Soffian wrote:
> Add log.abbrev-commit (and log.abbrevCommit synonym)

Why two options? It would be more conventional to provide just log.abbrevCommit. The existing "[add] ignore-errors" is explained in the manual to be a mistake and "[add] ignoreerrors" is the fixed version; adding more configuration variables with dashes in the name would seem to make guessing variable names even more hit-or-miss.

> config option as
> a convenience for users who often use --abbrev-commit with git log and
> git show (but not git whatchanged, as its output is more likely to be
> parsed even though it is not technically plumbing).

Hm, that wouldn't have been my hunch. Are you aware of any scripts that parse "git whatchanged" output?

More worrying is "git log --format=raw". I think as long as we're cautious about rolling this out slowly and noticing breakage early it should be okay. It might even be nice to find out if there are scripts or tests that care deeply about "git log"'s output format (which would be more reliable if they had been written to use "git rev-list | git diff-tree -s --stdin").

> Allow the option to be overridden via --no-abbrev-commit.

Good idea anyway. Once parse_revision_opt learns to use parse_options these negated options would be automatic (though that's a long way away).

Unfortunately this wouldn't help scripts much until the option has been around for a while. Maybe it would be safer to have two patches --- one to add --no-abbrev-commit which could be included in "maint" and widely deployed, and one to add the new configuration only after --no-abbrev-commit can be relied on? But on the other hand, scripts can be updated today to use rev-list | diff-tree, so maybe that's not worth the trouble.

People using git by hand would certainly appreciate --no-abbrev-commit, I suspect. Thanks for thinking about these things.

Show 11 quoted lines
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1314,6 +1314,12 @@ interactive.singlekey::
>  	linkgit:git-checkout[1]. Note that this setting is silently
>  	ignored if portable keystroke input is not available.
>  
> +log.abbrev-commit::
> +log.abbrevCommit::
> +	If true, act as if --abbrev-commit were specified on the command
> +	line. May be overridden with --no-abbrev-commit. Note that this setting
> +	is ignored by rev-list.
Style: most of that page is written from the point of view of the
user, like:
	Tells 'git apply' how to handle whitespace.  Set this to
	"ignore" if you don't want to be bothered.  See
	linkgit:git-apply[1] for details.
So maybe something like:
	Whether to abbreviate hexadecimal commit object names in
	output from the 'log' family of commands.  The number of
	digits shown is determined by the `--abbrev` command-line
	option and `core.abbrev` configuration variable.  Can be
	overridden on the command line by --abbrev-commit /
	--no-abbrev-commit.  The default is false.
 +
 This does not affect the 'git diff-tree' and 'git rev-list'
 commands.
Show 8 quoted lines
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -38,6 +38,9 @@ OPTIONS
>  	Continue listing the history of a file beyond renames
>  	(works only for a single file).
>  
> +--no-abbrev-commit::
> +	Don't abbreviate commit name. Useful for overridding log.abbrevCommit.
Also useful for overriding --abbrev-commit from aliases. :)
Shouldn't it be documented next to --abbrev-commit?
Show 9 quoted lines
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -23,6 +23,7 @@
>  /* Set a default date-time format for git log ("log.date" config variable) */
>  static const char *default_date_mode = NULL;
>  
> +static int default_abbrev_commit = 0;
>  static int default_show_root = 1;
>  static int decoration_style;
Style: we try to avoid unnecessary zero initializers for variables in
the BSS section.
[...]
Show 13 quoted lines
> @@ -89,11 +91,13 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>  			 struct rev_info *rev, struct setup_revision_opt *opt)
>  {
>  	struct userformat_want w;
> -	int quiet = 0, source = 0;
> +	int quiet = 0, source = 0, no_abbrev_commit = 0;
>  
>  	const struct option builtin_log_options[] = {
> -		OPT_BOOLEAN(0, "quiet", &quiet, "supress diff output"),
> +		OPT_BOOLEAN(0, "quiet", &quiet, "suppress diff output"),
>  		OPT_BOOLEAN(0, "source", &source, "show source"),
> +		OPT_BOOLEAN(0, "no-abbrev-commit", &no_abbrev_commit,
> +			    "don't abbreviate commit name"),
What happens if I do
	git log --no-abbrev-commit --abbrev-commit
?  How about
	git log --no-abbrev-commit --no-no-abbrev-commit --abbrev-commit
? :)  The behavior should be nicer if this is implemented in revision.c.
[...]
Show 6 quoted lines
> @@ -323,6 +330,11 @@ static int git_log_config(const char *var, const char *value, void *cb)
>  		return git_config_string(&fmt_pretty, var, value);
>  	if (!strcmp(var, "format.subjectprefix"))
>  		return git_config_string(&fmt_patch_subject_prefix, var, value);
> +	if (!strcasecmp(var, "log.abbrevcommit") ||
> +	    !strcasecmp(var, "log.abbrev-commit")) {

No need to use strcasecmp --- the vars passed to config functions already have the section and variable names in lowercase.

Show 5 quoted lines
> @@ -347,6 +359,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
>  	struct setup_revision_opt opt;
>  
>  	git_config(git_log_config, NULL);
> +	default_abbrev_commit = 0;
Puzzling as mentioned above.
Show 13 quoted lines
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -450,6 +450,14 @@ test_expect_success 'log.decorate configuration' '
>  
>  '
>  
> +test_expect_success 'log.abbrev-commit configuration' '
> +	test_might_fail git config --remove-section log &&
> +	git log --abbrev-commit >expect &&
> +	git config log.abbrev-commit true &&
> +	git log >actual &&
> +	test_cmp expect actual
> +'
To avoid polluting the configuration, it would be nicest to do:
	git config log.abbrev-commit true &&
	test_when_finished "git config --unset log.abbrev-commit" &&
	git log >actual &&
though it looks like some tests already protect themselves.
Just because I'm curious: what happens if you do
	git config log.abbrev-commit true

in test_create_repo in test-lib.sh? (I.e., are there many tests that would be confused by this?) Tests tend to be more picky than user scripts about the output of git but it might still be an ok way to get a vague sense of the impact.

Hope that helps, and thanks for a pleasant read.

Regards, Jonathan

Previous: Jay SoffianNext: Jay Soffian
Message 3 of 22 in “Add log.abbrev-commit config option”
  1. Add log.abbrev-commit config optionJay Soffian, May 14, 2011
  2. Jay SoffianMay 14, 2011
  3. Jonathan NiederMay 14, 2011
  4. Jay SoffianMay 14, 2011
  5. add, merge, diff: do not use strcasecmp to compare config variable namesJonathan Nieder, May 14, 2011
  6. Junio C HamanoMay 15, 2011
  7. Add log.abbrevCommit config variableJay Soffian, May 14, 2011
  8. Jonathan NiederMay 14, 2011
  9. Jay SoffianMay 14, 2011
  10. Add log.abbrevCommit config variableJay Soffian, May 14, 2011
  11. Jay SoffianMay 15, 2011
  12. Junio C HamanoMay 15, 2011
  13. Jay SoffianMay 16, 2011
  14. Jonathan NiederMay 16, 2011
  15. Jay SoffianMay 16, 2011
  16. Junio C HamanoMay 17, 2011
  17. Junio C HamanoMay 17, 2011
  18. Jay SoffianMay 18, 2011
  19. Junio C HamanoMay 17, 2011
  20. Jay SoffianMay 17, 2011
  21. Junio C HamanoMay 15, 2011
  22. Michael J GruberMay 16, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.