matni...@gmail.com writes:

> Subject: Re: [PATCH] Pretty-format: Ability to add newline after non-empty 
> string

Downcasing 'P' and 'A' would make this fit better when it appears in
the "git shortlog --no-merges" output, I think.  Or perhaps

        [PATCH] pretty: allow to add LF only after non-empty string

or something may fit even better.

> diff --git a/pretty.c b/pretty.c
> index 0ab45d10d7..fedea05acc 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1457,7 +1457,8 @@ static size_t format_commit_item(struct strbuf *sb, /* 
> in UTF-8 */
>               NO_MAGIC,
>               ADD_LF_BEFORE_NON_EMPTY,
>               DEL_LF_BEFORE_EMPTY,
> -             ADD_SP_BEFORE_NON_EMPTY
> +             ADD_SP_BEFORE_NON_EMPTY,
> +             ADD_LF_AFTER_NON_EMPTY

Appending at the end in this case does not make less sense than
inserting at the right place in the middle.  Noticing that earlier
ones are all about LF, perhaps

        NO_MAGIC
        ADD_LF_BEFORE_NON_EMPTY
        ADD_LF_AFTER_NON_EMPTY
        DEL_LF_BEFORE_EMPTY
        ADD_SP_BEFORE_NON_EMPTY

may make more sense?  

An obvious question that would come to the reader's mind would be if
we also want ADD_SP_AFTER and DEL_SP_BEFORE for completeness, once
the list is ordered in a more logical way like we see above.

I am not suggesting to add these two right now, but we still must
think about them before adding this new one, because it would affect
the choice of the letter used for this new one.  It is irresponsible
to say "I do not need it, so somebody else can add them later",
without making sure that somebody else can later choose sensible
letters that make the parallel between those two they are adding
with the existing ones.

We have '+' and '-' as a matching pair that adds or deletes a LF
before the expansion, and we are adding '*' to that group to make
the repertoire <add before non-empty, del before empty, add after
non-empty>.  On the SP side, we currently only use ' ', whose
counterpart on the LF side is '+'.  Can we easily find counterparts
for '-' and this new '*' for the SP side, when we want to add "add
after non-empty" and "delete before empty", or would it become
easier if we chose something other than '*', and if so what letter?

> @@ -1501,7 +1507,8 @@ static size_t userformat_want_item(struct strbuf *sb, 
> const char *placeholder,
>  {
>       struct userformat_want *w = context;
>  
> -     if (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ')
> +     if (*placeholder == '+' || *placeholder == '-' ||
> +             *placeholder == ' ' || *placeholder == '*')
>               placeholder++;
>  
>       switch (*placeholder) {
> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
> index da113d975b..e333ed91d3 100755
> --- a/t/t6006-rev-list-format.sh
> +++ b/t/t6006-rev-list-format.sh
> @@ -445,6 +445,11 @@ test_expect_success 'add SP before non-empty (2)' '
>       test $(wc -w <actual) = 6
>  '
>  
> +test_expect_success 'add LF after non-empty' '
> +     git show -s --pretty=format:"%s%*sThanks" HEAD^^ >actual &&

Here the subject is expanded, LF is added, and then the subject is
expanded _again_ before 'Thanks'.  That does not sound like a
realistic example that shows off the situation in which this new
feature becomes useful.

> +     test_line_count = 2 actual
> +'
> +
>  test_expect_success '--abbrev' '
>       echo SHORT SHORT SHORT >expect2 &&
>       echo LONG LONG LONG >expect3 &&

Thanks.

Reply via email to