Re: [PATCH 2/3 v3] Teach git-rev-list --ignore-merge-bases

From: Johannes Sixt <j6t@kdbg.org>
Date: May 2, 2020, 13:25 UTC
Message-ID: <37b157a3-f2d6-f459-2f2c-2ab53ba2f7ac@kdbg.org>
In-Reply-To: <1716d20d-6f0d-3872-cf36-6fc8d7bdb457@gmail.com>

Am 01.05.20 um 16:22 schrieb Antonio Russo:

Show 8 quoted lines

> +--ignore-merge-bases::
> +	Show commits that are introduced by each merge before showing
> +	the first parent of the merge but remove edges from those commits
> +	to commits reachable from the first parent. When used with
> +	`--graph`, this can help visualize repositories with many merges
> +	when you are not interested in the merge base used for each
> +	merge. It also reduces the width of the graph visualization
> +	when used with `--graph`.

I wonder whether there is some potential to prettify another aspect of --graph once this feature is in place: In particular, when I *am* interested in merge-bases, I use --boundary, and I get this chart:

$ git log --graph --boundary --oneline --no-decorate 7d28d69174~4..7d28d69174

*   7d28d69174 Merge branch 'jc/allow-strlen-substitution-in-shell-scripts'
|\  
| * 78725ebda9 CodingGuidelines: allow ${#posix} == strlen($posix)
* |   dfdce31ce6 Merge branch 'en/pull-do-not-rebase-after-fast-forwarding'
|\ \  
| * | fbae70ddc6 pull: avoid running both merge and rebase
| |/  
* |   b660a76d0f Merge branch 'dl/wrapper-fix-indentation'
|\ \  
| * | 7cd54d37dc wrapper: indent with tabs
* | |   3d6c56dd66 Merge branch 'ag/sequencer-i18n-messages'
|\ \ \  
| * | | 4d55d63bde sequencer: mark messages for translation
| o | | 3bab5d5625 The second batch post 2.26 cycle
|  / /  
o / / 9f471e4b95 Merge branch 'rs/pull-options-sync-code-and-doc'
 / /  
o / 0b6806b9e4 xread, xwrite: limit size of IO to 8MB
 /  
o b6d4d82bd5 msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.x

but I would prefer to see something like this:

*   7d28d69174 Merge branch 'jc/allow-strlen-substitution-in-shell-scripts'
|\  
| * 78725ebda9 CodingGuidelines: allow ${#posix} == strlen($posix)
* |   dfdce31ce6 Merge branch 'en/pull-do-not-rebase-after-fast-forwarding'
|\ \  
| * | fbae70ddc6 pull: avoid running both merge and rebase
| |/
| o b6d4d82bd5 msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.x  
*   b660a76d0f Merge branch 'dl/wrapper-fix-indentation'
|\  
| * 7cd54d37dc wrapper: indent with tabs
| o 0b6806b9e4 xread, xwrite: limit size of IO to 8MB
*   3d6c56dd66 Merge branch 'ag/sequencer-i18n-messages'
|\  
| * 4d55d63bde sequencer: mark messages for translation
| o 3bab5d5625 The second batch post 2.26 cycle
o 9f471e4b95 Merge branch 'rs/pull-options-sync-code-and-doc'

Maybe that is just an easy fall-out?

Show 10 quoted lines

> diff --git a/object.h b/object.h
> index b22328b838..0bf6fb0d55 100644
> --- a/object.h
> +++ b/object.h
> @@ -59,7 +59,7 @@ struct object_array {
> 
>  /*
>   * object flag allocation:
> - * revision.h:               0---------10         15                   25----28
> + * revision.h:               0---------10         15                   23----28

I think the intent is that the line is like a ruler with marks: you should move "23" sufficiently far to the left, because the "23" mark cannot be at the same spot where the "25" mark was.

(I'm not reviewing this patch as I am illiterate when it comes to the revision walker, I just happened to notice this when I skimmed the patch.)

>   * fetch-pack.c:             01
>   * negotiator/default.c:       2--5
>   * walker.c:                 0-2

-- Hannes