Re: [PATCH 10/11] cal: trim three month display trailing white spaces

["Pádraig Brady" <P@draigBrady.com>]

On 04/29/2013 07:01 PM, Sami Kerola wrote:
> On 29 April 2013 14:40, Pádraig Brady <P@draigbrady.com> wrote:
>> On 04/29/2013 10:11 AM, Sami Kerola wrote:
>>> On 29 April 2013 01:32, Pádraig Brady <P@draigbrady.com> wrote:
>>>> On 04/28/2013 06:11 PM, Sami Kerola wrote:
>>>>> Includes fix to checks that conform earlier output practise.
>>>>
>>>> I'm not sure about this.
>>>> Other output modes have trailing whitespace.
>>>> Also consider the case where you're outputting to a <pre> in a web page,
>>>> where the pre has a different bg color to the web page.
>>>> You'd want the full width output in that case.
>>>
>>> I did not realize someone might be doing that.  The reason why I wrote
>>> the change is to be consistent with month and year print out, which
>>> are already trimming spaces.
>>
>> I don't see month/year here with trimmed spaces?
>>
>> $ cal -1 | head -n1 | tr ' ' .
>> .....April.2013.....
>>
>> $ cal -y | head -n3 | tr ' ' .
>> ...............................2013...............................
>>
>> .......January...............February.................March.......
>>
>> I do see that spaces are trimmed from the last day in -1 and -y formats.
>> So I suppose it's best to be consistent and trail space there.
>> I suppose one could also not trim spaces for -1 and -y there?
> 
> Headers are using mbsalign() function to align center, which also does
> printing. That printing is not filtered through trim_trailing_spaces()
> like the calendar's numeric output.

As an aside, the "upstream" for mbsalign() in coreutils
has been updated to support MBA_NO_RIGHT_PAD.  If we were to change
that behaviour, we would probably sync up first.
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=6cf9c59b

> 
>>> Perhaps there should be
>>> '--trim=[yes|no|auto]', where the 'auto' would be the current
>>> practice, and default.
>>
>> I don't think it needs an option TBH as removing trailing
>> spaces is trivial with: sed 's/ *$//'
>> Padding out with spaces is not trivial though,
>> which is another reason to err on the side of
>> leaving the trailing spaces.
> 
> True. Perhaps dropping the trimming all together is correct thing to
> do. The following demonstrates what not trimming would mean for code
> and tests.

+1

thanks,
Pádraig.