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

Re: [RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list

From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date
Jul 28, 2015, 13:01 UTC
Message-ID
<vpqlhe0xwpv.fsf@anie.imag.fr>
In-Reply-To
<1438066594-5620-6-git-send-email-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> Remove show_detached() and make detached HEAD to be rolled into
> regular ref_list by adding REF_DETACHED_HEAD as a kind of branch and
> supporting the same in append_ref().
Again, this lacks the "why?" explanation.
> Bump get_head_description() to the top.

Here also: why? And this could easily go to a separate patch to let the reviewer focus on actual changes.

Show 15 quoted lines
> @@ -504,6 +540,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>  	int color;
>  	struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
>  	const char *prefix = "";
> +	const char *desc = item->name;
>  
>  	if (item->ignore)
>  		return;
> @@ -516,6 +553,10 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>  		color = BRANCH_COLOR_REMOTE;
>  		prefix = remote_prefix;
>  		break;
> +	case REF_DETACHED_HEAD:
> +		color = BRANCH_COLOR_CURRENT;
> +		desc = get_head_description();

I think you're leaking a string here: get_head_description() builds an strbuf and returns the dynamically allocated string, which you never free.

Show 5 quoted lines
> -static void show_detached(struct ref_list *ref_list, int maxwidth)
> -{
> -	struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1);
> -
> -	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {

I'm not sure what this if was doing, and why you can get rid of it. My understanding is that with_commit comes from --contains, and in the previous code the filtering was done at display time (detached HEAD was not shown if it was not contained in commits specified with --contains).

Eventually, you'll use ref-filter to do this filtering so you won't need this check at display time.

But am I correct that for a few commits, you ignore --contains on detached HEAD?

Show 8 quoted lines
> @@ -678,15 +674,20 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
>  	if (verbose)
>  		maxwidth = calc_maxwidth(&ref_list, strlen(remote_prefix));
>  
> -	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
> +	index = ref_list.index;
> +
> +	/* Print detached heads before sorting and printing the rest */
I think you mean head (no s) or HEAD. It's not Mercurial ;-).
Show 6 quoted lines
> +	if (detached && (ref_list.list[index - 1].kind == REF_DETACHED_HEAD) &&
> +	    !strcmp(ref_list.list[index - 1].name, head)) {
> +		print_ref_item(&ref_list.list[index - 1], maxwidth, verbose, abbrev,
> +			       1, remote_prefix);
> +		index -= 1;
> +	}

This relies on the assumption that HEAD has to be the last in the array. The assumption is correct since you call head_ref(append_ref, &cb) after for_each_rawref(append_ref, &cb). I think this deserves a comment to remind the reader that HEAD is always the last (here or at the call to head_ref to make sure nobody try to change the order between head_ref and for_each_rawref).

It may be more natural to do it the other way around: call head_ref first and get HEAD first, as you are going to display it first (but in any case, you'll have to call qsort on a subset of the array so it doesn't change much).

Show 8 quoted lines
> @@ -913,7 +914,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  			die(_("branch name required"));
>  		return delete_branches(argc, argv, delete > 1, kinds, quiet);
>  	} else if (list) {
> -		int ret = print_ref_list(kinds, detached, verbose, abbrev,
> +		int ret;
> +		if (kinds & REF_LOCAL_BRANCH)
> +			kinds |= REF_DETACHED_HEAD;
Perhaps add
	/* git branch --local also shows HEAD when it is detached */
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Previous: Karthik NayakNext: Karthik Nayak
Message 34 of 88 in “Port branch.c to use ref-filter APIs”
  1. Port branch.c to use ref-filter APIsKarthik Nayak, Jul 28, 2015
  2. 01/11 ref-filter: add "%(objectname:size=X)" optionKarthik Nayak, Jul 28, 2015
  3. 02/11 ref-filter: add 'colornext' atomKarthik Nayak, Jul 28, 2015
  4. Matthieu MoyJul 28, 2015
  5. Karthik NayakJul 28, 2015
  6. Christian CouderJul 28, 2015
  7. Karthik NayakJul 28, 2015
  8. Eric SunshineJul 29, 2015
  9. Matthieu MoyJul 29, 2015
  10. Jacob KellerJul 30, 2015
  11. Junio C HamanoJul 30, 2015
  12. Karthik NayakAug 1, 2015
  13. 03/11 ref-filter: add option to filter only branchesKarthik Nayak, Jul 28, 2015
  14. Matthieu MoyJul 28, 2015
  15. Karthik NayakJul 28, 2015
  16. 04/11 ref-filter: add 'ifexists' atomKarthik Nayak, Jul 28, 2015
  17. Jacob KellerJul 28, 2015
  18. Karthik NayakJul 28, 2015
  19. Matthieu MoyJul 28, 2015
  20. Karthik NayakJul 28, 2015
  21. Junio C HamanoJul 28, 2015
  22. Karthik NayakJul 29, 2015
  23. Junio C HamanoJul 29, 2015
  24. Junio C HamanoJul 29, 2015
  25. Matthieu MoyJul 29, 2015
  26. Junio C HamanoJul 29, 2015
  27. Karthik NayakAug 1, 2015
  28. Jacob KellerAug 1, 2015
  29. Karthik NayakAug 1, 2015
  30. 05/11 branch: fix width computationKarthik Nayak, Jul 28, 2015
  31. Matthieu MoyJul 28, 2015
  32. Karthik NayakJul 28, 2015
  33. 06/11 branch: roll show_detached HEAD into regular ref_listKarthik Nayak, Jul 28, 2015
  34. Matthieu MoyJul 28, 2015
  35. Karthik NayakJul 28, 2015
  36. Matthieu MoyJul 29, 2015
  37. Karthik NayakJul 29, 2015
  38. 07/11 branch: move 'current' check down to the presentation layerKarthik Nayak, Jul 28, 2015
  39. Matthieu MoyJul 28, 2015
  40. Karthik NayakJul 28, 2015
  41. Jacob KellerJul 29, 2015
  42. Karthik NayakJul 29, 2015
  43. Matthieu MoyJul 29, 2015
  44. Karthik NayakJul 29, 2015
  45. Matthieu MoyJul 29, 2015
  46. Karthik NayakAug 1, 2015
  47. Jacob KellerAug 1, 2015
  48. Eric SunshineAug 1, 2015
  49. Karthik NayakAug 2, 2015
  50. Junio C HamanoAug 2, 2015
  51. 08/11 branch: drop non-commit error reportingKarthik Nayak, Jul 28, 2015
  52. 09/11 branch.c: use 'ref-filter' data structuresKarthik Nayak, Jul 28, 2015
  53. Christian CouderJul 28, 2015
  54. Matthieu MoyJul 28, 2015
  55. Karthik NayakJul 28, 2015
  56. Matthieu MoyJul 29, 2015
  57. Karthik NayakJul 29, 2015
  58. Karthik NayakJul 28, 2015
  59. Christian CouderJul 28, 2015
  60. Karthik NayakJul 28, 2015
  61. Matthieu MoyJul 28, 2015
  62. Karthik NayakJul 28, 2015
  63. Junio C HamanoJul 28, 2015
  64. Karthik NayakJul 28, 2015
  65. Junio C HamanoJul 28, 2015
  66. Karthik NayakJul 29, 2015
  67. 10/11 branch.c: use 'ref-filter' APIsKarthik Nayak, Jul 28, 2015
  68. Jacob KellerJul 28, 2015
  69. Karthik NayakJul 29, 2015
  70. Christian CouderJul 28, 2015
  71. Karthik NayakJul 29, 2015
  72. Matthieu MoyJul 28, 2015
  73. Karthik NayakJul 29, 2015
  74. Matthieu MoyJul 29, 2015
  75. Karthik NayakJul 29, 2015
  76. Matthieu MoyJul 29, 2015
  77. Karthik NayakJul 30, 2015
  78. Matthieu MoyJul 30, 2015
  79. Karthik NayakAug 3, 2015
  80. Matthieu MoyAug 19, 2015
  81. Karthik NayakAug 19, 2015
  82. 11/11 branch: add '--points-at' optionKarthik Nayak, Jul 28, 2015
  83. Jacob KellerJul 28, 2015
  84. Karthik NayakJul 29, 2015
  85. Matthieu MoyJul 28, 2015
  86. Karthik NayakJul 28, 2015
  87. Matthieu MoyJul 28, 2015
  88. Karthik NayakJul 29, 2015

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.