On 29/12/12 01:18, Florian Pritz wrote:
> On 20.12.2012 11:31, Allan McRae wrote:
>> On 20/12/12 10:19, Dave Reisner wrote:
>>> On Thu, Dec 20, 2012 at 01:06:36AM +0100, Florian Pritz wrote:
>>>> Signed-off-by: Florian Pritz <[email protected]>
>>>> ---
>>>> Now uses printf instead of echo -n.
>>>>
>>>>  contrib/pacdiff.sh.in | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in
>>>> index 86dc20e..3461627 100644
>>>> --- a/contrib/pacdiff.sh.in
>>>> +++ b/contrib/pacdiff.sh.in
>>>> @@ -97,15 +97,16 @@ while IFS= read -u 3 -r -d '' pacfile; do
>>>>            echo "  Files are identical, removing..."
>>>>            rm -v "$pacfile"
>>>>    else
>>>> -          echo -n "  File differences found. (V)iew, (S)kip, (R)emove: 
>>>> [v/s/r] "
>>>> +          printf "  (V)iew, (S)kip, (R)emove $file_type, (O)verwrite with 
>>>> $file_type: [v/s/r/o] "
>>>
>>> You're using the prompt as a format string. It's obvious what the
>>> possible values for $file_type are, but I'd rather these be inserted as
>>> tokens replacements rather than embedded in the format string.
> 
> @Dave: I've fixed that in my repo.
> 
>> I am more concerned that the prompt line will almost always go over 80
>> characters.
> 
> @Allan:
> The longest prompt line possible (file_type = pacorig or pacsave) is 72
> chars including all spaces:
> "  (V)iew, (S)kip, (R)emove pacorig, (O)verwrite with pacorig: [v/s/r/o] "
> 
> Make that 73 if you want to count the char the user has to enter.
> 
>> How about"
>>
>> printf "  File differences found in %s\n" $file_type
>> printf "  (V)iew, (S)kip, (R)emove, (O)verwrite: [v/s/r/o] "
> 
> That doesn't mention what file will be removed (or overwritten) which I
> found pretty disturbing in the original script and which cause me to
> write all those patches.
> 
> If you think a little bit about it the only sane case is to remove the
> pacsave, pacnew, pacorig, but I still think being ambiguous here is a
> bad idea because we are dealing mostly with important files and users
> should be reassured that we are doing the right thing and they don't
> have to worry.
> 

ah - it is just the extension printed.  I thought it was the whole
filename.   (I will assume the whole filename is printed above this
somewhere...  too lazy to check!)

Patchset looks fine to me.



Reply via email to