Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Christian Couder]

On Fri, Aug 11, 2017 at 11:06 AM, Jeff King  wrote:
> On Fri, Aug 11, 2017 at 09:02:24AM +0200, Christian Couder wrote:
>
>> > But I really don't want callers to think of it as "unfold". I want it to
>> > be "turn this into something I can parse simply". Hence if we were to
>> > find another case where the output is irregular, I'd feel comfortable
>> > calling that a bug and fixing it (one that I suspect but haven't tested
>> > is that alternate separators probably should all be converted to
>> > colons).
>>
>> Though "fixing" this after a release has been made might introduce a
>> regression for people who would already use the option on trailers
>> with different separators. That's also why I don't like --normalize.
>> We just don't know if we will need to "fix" it a lot to make sure
>> scripts using it will work in all cases.
>>
>> If we use --no-fold or --oneline instead, we don't promise too much
>> from this option, so users cannot say that they expect it to work for
>> them in all cases.
>
> But promising a normalized form is exactly what I want from the option.
>
> That said, I'm OK to promise that via "--parse", and call this --unfold,
> if you really feel strongly.

Yeah, I think promising these kind of things via an higher level
option that is a shorthand for a mix of other basic options is much
better especially if it's clearly documented that the option mix could
change in case of bugs or improvements.

This way people who want something stable, know that they should use
their own mix of basic options. And those who are ok with something
not as stable as long as it evolves towards a specific goal, know that
they should use the higher level option.

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Jeff King]

On Fri, Aug 11, 2017 at 09:02:24AM +0200, Christian Couder wrote:

> > But I really don't want callers to think of it as "unfold". I want it to
> > be "turn this into something I can parse simply". Hence if we were to
> > find another case where the output is irregular, I'd feel comfortable
> > calling that a bug and fixing it (one that I suspect but haven't tested
> > is that alternate separators probably should all be converted to
> > colons).
> 
> Though "fixing" this after a release has been made might introduce a
> regression for people who would already use the option on trailers
> with different separators. That's also why I don't like --normalize.
> We just don't know if we will need to "fix" it a lot to make sure
> scripts using it will work in all cases.
> 
> If we use --no-fold or --oneline instead, we don't promise too much
> from this option, so users cannot say that they expect it to work for
> them in all cases.

But promising a normalized form is exactly what I want from the option.

That said, I'm OK to promise that via "--parse", and call this --unfold,
if you really feel strongly.

-Peff

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Christian Couder]

On Fri, Aug 11, 2017 at 1:10 AM, Jeff King  wrote:
> On Fri, Aug 11, 2017 at 12:02:49AM +0100, Ramsay Jones wrote:
>
>> > But some of those things are not 1:1 mappings with normalization.  For
>> > instance, --json presumably implies --only-trailers. Or are we proposing
>> > to break the whole commit message down into components and output it all
>> > as json?

Well who knows what people might want/need?
Anyway in `git log` --oneline is not a direct mapping with
--pretty=oneline as it also means --abbrev-commit, and this is not a
big problem.

>> Hmm, to me the operation wasn't so much a normalization, rather
>> it was an --unfold (or maybe --un-fold ;-D). I suppose going in
>> the other direction could be --fold=72, or some such.

Yeah, we could call that --no-fold or --no-wrap if we expect to need
--fold=72 or --wrap=72.
At least it is more descriptive than --normalize and if we later
introduce --pretty or --format we can say that it is a shorthand for
--pretty=nofold or --pretty=unfolded.

> But I really don't want callers to think of it as "unfold". I want it to
> be "turn this into something I can parse simply". Hence if we were to
> find another case where the output is irregular, I'd feel comfortable
> calling that a bug and fixing it (one that I suspect but haven't tested
> is that alternate separators probably should all be converted to
> colons).

Though "fixing" this after a release has been made might introduce a
regression for people who would already use the option on trailers
with different separators. That's also why I don't like --normalize.
We just don't know if we will need to "fix" it a lot to make sure
scripts using it will work in all cases.

If we use --no-fold or --oneline instead, we don't promise too much
from this option, so users cannot say that they expect it to work for
them in all cases.

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Ramsay Jones]

On 11/08/17 00:10, Jeff King wrote:
> On Fri, Aug 11, 2017 at 12:02:49AM +0100, Ramsay Jones wrote:
> 
>>> But some of those things are not 1:1 mappings with normalization.  For
>>> instance, --json presumably implies --only-trailers. Or are we proposing
>>> to break the whole commit message down into components and output it all
>>> as json?
>>
>> Hmm, to me the operation wasn't so much a normalization, rather
>> it was an --unfold (or maybe --un-fold ;-D). I suppose going in
>> the other direction could be --fold=72, or some such.
> 
> But I really don't want callers to think of it as "unfold". I want it to
> be "turn this into something I can parse simply". Hence if we were to
> find another case where the output is irregular, I'd feel comfortable
> calling that a bug and fixing it (one that I suspect but haven't tested
> is that alternate separators probably should all be converted to
> colons).

Ah, yes, good point.

ATB,
Ramsay Jones

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Jeff King]

On Fri, Aug 11, 2017 at 12:02:49AM +0100, Ramsay Jones wrote:

> > But some of those things are not 1:1 mappings with normalization.  For
> > instance, --json presumably implies --only-trailers. Or are we proposing
> > to break the whole commit message down into components and output it all
> > as json?
> 
> Hmm, to me the operation wasn't so much a normalization, rather
> it was an --unfold (or maybe --un-fold ;-D). I suppose going in
> the other direction could be --fold=72, or some such.

But I really don't want callers to think of it as "unfold". I want it to
be "turn this into something I can parse simply". Hence if we were to
find another case where the output is irregular, I'd feel comfortable
calling that a bug and fixing it (one that I suspect but haven't tested
is that alternate separators probably should all be converted to
colons).

> [blue is my favourite colour ... :-P ]

Yes, I'm feeling that, too. :-/

-Peff

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Ramsay Jones]

On 10/08/17 22:10, Jeff King wrote:
> On Thu, Aug 10, 2017 at 11:06:40PM +0200, Christian Couder wrote:
> 
 Related to this, I wonder if people might want to "normalize" in
 different ways later. If that happens, we might regret having called
 this option "--normalize" instead of "--one-per-line" for example.
>>>
>>> What is normal?
>>>
>>> Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]
>>> then?
>>
>> Yeah, we could go there right now (using perhaps "--pretty" or
>> "--format" instead of "--style", so that we are more consistent with
>> other commands), but on the other hand we don't know yet if the other
>> formats will really be needed.
> 
> But some of those things are not 1:1 mappings with normalization.  For
> instance, --json presumably implies --only-trailers. Or are we proposing
> to break the whole commit message down into components and output it all
> as json?

Hmm, to me the operation wasn't so much a normalization, rather
it was an --unfold (or maybe --un-fold ;-D). I suppose going in
the other direction could be --fold=72, or some such.

[blue is my favourite colour ... :-P ]

ATB,
Ramsay Jones

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Jeff King]

On Thu, Aug 10, 2017 at 11:06:40PM +0200, Christian Couder wrote:

> >> Related to this, I wonder if people might want to "normalize" in
> >> different ways later. If that happens, we might regret having called
> >> this option "--normalize" instead of "--one-per-line" for example.
> >
> > What is normal?
> >
> > Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]
> > then?
> 
> Yeah, we could go there right now (using perhaps "--pretty" or
> "--format" instead of "--style", so that we are more consistent with
> other commands), but on the other hand we don't know yet if the other
> formats will really be needed.

But some of those things are not 1:1 mappings with normalization.  For
instance, --json presumably implies --only-trailers. Or are we proposing
to break the whole commit message down into components and output it all
as json?

-Peff

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Christian Couder]

On Thu, Aug 10, 2017 at 9:44 PM, Stefan Beller  wrote:
> On Thu, Aug 10, 2017 at 12:39 PM, Christian Couder
>  wrote:
>> On Thu, Aug 10, 2017 at 8:37 PM, Jeff King  wrote:
>>> On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:
>>>
 On Thu, Aug 10, 2017 at 1:03 AM, Jeff King  wrote:
 > The point of "--only-trailers" is to give a caller an output
 > that's easy for them to parse. Getting rid of the
 > non-trailer material helps, but we still may see more
 > complicated syntax like whitespace continuation. Let's add
 > an option to normalize the output into one "key: value" line
 > per trailer.
 >
 > As a bonus, this could be used even without --only-trailers
 > to clean up unusual formatting in the incoming data.

 This is useful for the parsing part, but for the writing part we'd
 rather want to have the opposite thing, such as
 '--line-break=rfc822'. But this doesn't have to be part of this
 series. With this in mind, I do not quite understand the latter
 use case how you would use normalized trailers without
 --only-trailers?
>>>
>>> If you prefer the normalized form (and the input was line-broken in a
>>> way that you don't like), then this would convert to your preferred
>>> form. I agree that you could potentially want the opposite (folding long
>>> lines). Perhaps something like --wrap=72.
>>
>> Related to this, I wonder if people might want to "normalize" in
>> different ways later. If that happens, we might regret having called
>> this option "--normalize" instead of "--one-per-line" for example.
>
> What is normal?
>
> Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]
> then?

Yeah, we could go there right now (using perhaps "--pretty" or
"--format" instead of "--style", so that we are more consistent with
other commands), but on the other hand we don't know yet if the other
formats will really be needed.

> If you have --one-per-line, this may be orthogonal to e.g. json
> (as json can be crammed into one line IIUC), but when given the
> selection you cannot combine multiple styles.
>
> Scratch that, we actually want to combine these styles with each
> other.

Yeah, that's another possibility for the future. People might want a
--json option that can be used both with and without --oneline. But as
the future is difficult to predict, let's try to make it easy for us
in both cases.

And I think starting with just "--oneline" would be easier to deal
with later than "--normalize" (or "--style" or "--pretty" or
"--format") especially in the latter case.

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Christian Couder]

On Thu, Aug 10, 2017 at 9:42 PM, Jeff King  wrote:
> On Thu, Aug 10, 2017 at 09:39:21PM +0200, Christian Couder wrote:
>
>> > If you prefer the normalized form (and the input was line-broken in a
>> > way that you don't like), then this would convert to your preferred
>> > form. I agree that you could potentially want the opposite (folding long
>> > lines). Perhaps something like --wrap=72.
>>
>> Related to this, I wonder if people might want to "normalize" in
>> different ways later. If that happens, we might regret having called
>> this option "--normalize" instead of "--one-per-line" for example.
>
> My assumption was that it would be OK to add other normalization later
> if it brings us closer to the "key: value" form as a standard, and it
> could fall under "--normalize", since that's what callers would want.
> And that's why I didn't want to call it something like --one-per-line.
>
> But if you are arguing that there can be many "standards" to normalize
> to, I agree that's a possibility. I think we have an out by extending to
> "--normalize=whatever-form" in the future.

If we take `git log` as an example, we now have "--oneline" which is a
shorthand for "--pretty=oneline --abbrev-commit".
And the default for "--pretty" is called "medium".

So instead of your suggestion, we could call this option "--oneline"
now, and if other normalizations are later required we could then
create "--pretty=whatever" and say that "--oneline" is a shorthand for
"--pretty=oneline".

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Stefan Beller]

On Thu, Aug 10, 2017 at 12:39 PM, Christian Couder
 wrote:
> On Thu, Aug 10, 2017 at 8:37 PM, Jeff King  wrote:
>> On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:
>>
>>> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King  wrote:
>>> > The point of "--only-trailers" is to give a caller an output
>>> > that's easy for them to parse. Getting rid of the
>>> > non-trailer material helps, but we still may see more
>>> > complicated syntax like whitespace continuation. Let's add
>>> > an option to normalize the output into one "key: value" line
>>> > per trailer.
>>> >
>>> > As a bonus, this could be used even without --only-trailers
>>> > to clean up unusual formatting in the incoming data.
>>>
>>> This is useful for the parsing part, but for the writing part we'd
>>> rather want to have the opposite thing, such as
>>> '--line-break=rfc822'. But this doesn't have to be part of this
>>> series. With this in mind, I do not quite understand the latter
>>> use case how you would use normalized trailers without
>>> --only-trailers?
>>
>> If you prefer the normalized form (and the input was line-broken in a
>> way that you don't like), then this would convert to your preferred
>> form. I agree that you could potentially want the opposite (folding long
>> lines). Perhaps something like --wrap=72.
>
> Related to this, I wonder if people might want to "normalize" in
> different ways later. If that happens, we might regret having called
> this option "--normalize" instead of "--one-per-line" for example.

What is normal?

Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]
then?

If you have --one-per-line, this may be orthogonal to e.g. json
(as json can be crammed into one line IIUC), but when given the
selection you cannot combine multiple styles.

Scratch that, we actually want to combine these styles with each
other.

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Jeff King]

On Thu, Aug 10, 2017 at 09:39:21PM +0200, Christian Couder wrote:

> > If you prefer the normalized form (and the input was line-broken in a
> > way that you don't like), then this would convert to your preferred
> > form. I agree that you could potentially want the opposite (folding long
> > lines). Perhaps something like --wrap=72.
> 
> Related to this, I wonder if people might want to "normalize" in
> different ways later. If that happens, we might regret having called
> this option "--normalize" instead of "--one-per-line" for example.

My assumption was that it would be OK to add other normalization later
if it brings us closer to the "key: value" form as a standard, and it
could fall under "--normalize", since that's what callers would want.
And that's why I didn't want to call it something like --one-per-line.

But if you are arguing that there can be many "standards" to normalize
to, I agree that's a possibility. I think we have an out by extending to
"--normalize=whatever-form" in the future.

-Peff

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Christian Couder]

On Thu, Aug 10, 2017 at 8:37 PM, Jeff King  wrote:
> On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:
>
>> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King  wrote:
>> > The point of "--only-trailers" is to give a caller an output
>> > that's easy for them to parse. Getting rid of the
>> > non-trailer material helps, but we still may see more
>> > complicated syntax like whitespace continuation. Let's add
>> > an option to normalize the output into one "key: value" line
>> > per trailer.
>> >
>> > As a bonus, this could be used even without --only-trailers
>> > to clean up unusual formatting in the incoming data.
>>
>> This is useful for the parsing part, but for the writing part we'd
>> rather want to have the opposite thing, such as
>> '--line-break=rfc822'. But this doesn't have to be part of this
>> series. With this in mind, I do not quite understand the latter
>> use case how you would use normalized trailers without
>> --only-trailers?
>
> If you prefer the normalized form (and the input was line-broken in a
> way that you don't like), then this would convert to your preferred
> form. I agree that you could potentially want the opposite (folding long
> lines). Perhaps something like --wrap=72.

Related to this, I wonder if people might want to "normalize" in
different ways later. If that happens, we might regret having called
this option "--normalize" instead of "--one-per-line" for example.

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Jeff King]

On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:

> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King  wrote:
> > The point of "--only-trailers" is to give a caller an output
> > that's easy for them to parse. Getting rid of the
> > non-trailer material helps, but we still may see more
> > complicated syntax like whitespace continuation. Let's add
> > an option to normalize the output into one "key: value" line
> > per trailer.
> >
> > As a bonus, this could be used even without --only-trailers
> > to clean up unusual formatting in the incoming data.
> 
> This is useful for the parsing part, but for the writing part we'd
> rather want to have the opposite thing, such as
> '--line-break=rfc822'. But this doesn't have to be part of this
> series. With this in mind, I do not quite understand the latter
> use case how you would use normalized trailers without
> --only-trailers?

If you prefer the normalized form (and the input was line-broken in a
way that you don't like), then this would convert to your preferred
form. I agree that you could potentially want the opposite (folding long
lines). Perhaps something like --wrap=72.

-Peff

Re: [PATCH 4/5] interpret-trailers: add an option to normalize output

[Stefan Beller]

On Thu, Aug 10, 2017 at 1:03 AM, Jeff King  wrote:
> The point of "--only-trailers" is to give a caller an output
> that's easy for them to parse. Getting rid of the
> non-trailer material helps, but we still may see more
> complicated syntax like whitespace continuation. Let's add
> an option to normalize the output into one "key: value" line
> per trailer.
>
> As a bonus, this could be used even without --only-trailers
> to clean up unusual formatting in the incoming data.

This is useful for the parsing part, but for the writing part we'd
rather want to have the opposite thing, such as
'--line-break=rfc822'. But this doesn't have to be part of this
series. With this in mind, I do not quite understand the latter
use case how you would use normalized trailers without
--only-trailers?