Re: [PATCH v3 0/3] rebase-merges: try and use branch names for labels

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Nicolas

Thanks for working on this. This version looks good to me

Thanks

Phillip

On 09/10/2024 08:58, Nicolas Guichard via GitGitGadget wrote:
> When interactively rebasing merge commits, the commit message is parsed to
> extract a probably meaningful label name. For instance if the merge commit
> is “Merge branch 'feature0'”, then the rebase script will have thes lines:
> 
> label feature0
> merge -C $sha feature0 # “Merge branch 'feature0'
> 
> 
> This heuristic fails in the case of octopus merges or when the merge commit
> message is actually unrelated to the parent commits.
> 
> An example that combines both is:
> 
> *---.   967bfa4 (HEAD -> integration) Integration
> |\ \ \
> | | | * 2135be1 (feature2, feat2) Feature 2
> | |_|/
> |/| |
> | | * c88b01a Feature 1
> | |/
> |/|
> | * 75f3139 (feat0) Feature 0
> |/
> * `25c86d0` (main) Initial commit
> 
> 
> which yields the labels Integration, Integration-2 and Integration-3.
> 
> Fix this by using a branch name for each merge commit's parent that is the
> tip of at least one branch, and falling back to a label derived from the
> merge commit message otherwise. In the example above, the labels become
> feat0, Integration and feature2.
> 
> Changes since v1:
> 
>   * moved load_branch_decorations to re-use the decoration_loaded guard and
>     avoid pointlessly appending "refs/heads/" to a static string_list, as
>     pointed out by Junio C Hamano (thanks!)
>   * fixed a leak in load_branch_decorations found by making the filter
>     string_lists non-static
> 
> Changes since v2:
> 
>   * style changes (true/false -> 1/0 and // -> /* */)
> 
> Nicolas Guichard (3):
>    load_branch_decorations: fix memory leak with non-static filters
>    rebase-update-refs: extract load_branch_decorations
>    rebase-merges: try and use branch names as labels
> 
>   log-tree.c                    | 26 +++++++++++++++++++++++++
>   log-tree.h                    |  1 +
>   sequencer.c                   | 36 +++++++++++++++++------------------
>   t/t3404-rebase-interactive.sh |  4 ++--
>   t/t3430-rebase-merges.sh      | 12 ++++++------
>   5 files changed, 53 insertions(+), 26 deletions(-)
> 
> 
> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1784%2Fnicolas-guichard%2Fuse-branch-names-for-rebase-merges-labels-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1784/nicolas-guichard/use-branch-names-for-rebase-merges-labels-v3
> Pull-Request: https://github.com/git/git/pull/1784
> 
> Range-diff vs v2:
> 
>   1:  6250a7f6d6c ! 1:  e030ddd91f3 load_branch_decorations: fix memory leak with non-static filters
>       @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flag
>         				normalize_glob_ref(item, NULL, item->string);
>         			}
>        +
>       -+			// normalize_glob_ref duplicates the strings
>       -+			filter->exclude_ref_pattern->strdup_strings = true;
>       -+			filter->include_ref_pattern->strdup_strings = true;
>       -+			filter->exclude_ref_config_pattern->strdup_strings = true;
>       ++			/* normalize_glob_ref duplicates the strings */
>       ++			filter->exclude_ref_pattern->strdup_strings = 1;
>       ++			filter->include_ref_pattern->strdup_strings = 1;
>       ++			filter->exclude_ref_config_pattern->strdup_strings = 1;
>         		}
>         		decoration_loaded = 1;
>         		decoration_flags = flags;
>   2:  167418d10d1 ! 2:  1dad6096eb7 rebase-update-refs: extract load_branch_decorations
>       @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flag
>        +		string_list_append(&decorate_refs_include, "refs/heads/");
>        +		load_ref_decorations(&decoration_filter, 0);
>        +
>       -+		string_list_clear(&decorate_refs_exclude, false);
>       -+		string_list_clear(&decorate_refs_exclude_config, false);
>       -+		string_list_clear(&decorate_refs_include, false);
>       ++		string_list_clear(&decorate_refs_exclude, 0);
>       ++		string_list_clear(&decorate_refs_exclude_config, 0);
>       ++		string_list_clear(&decorate_refs_include, 0);
>        +	}
>        +}
>        +
>   3:  dfa1f0a7648 = 3:  19d8253da07 rebase-merges: try and use branch names as labels
>