threads / patch / 46966

patchsequencer.c: unify error messages

Subject: [PATCH] sequencer.c: unify error messages

## tl;dr

9 messages between Oct 13, 2017 and Oct 16, 2017. Diffs are folded; open one to read it.

replies: 8people: 5as markdown or json

Ralf Thielow· Oct 13, 2017, 17:51 UTC · lore

When ftruncate() in rearrange_squash() fails, we write that we couldn't finish the operation on the todo file. It is more accurate to write that we couldn't truncate as we do in other calls of ftruncate().

While at there, remove a full stop in another error message.
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to sequencer.c +2 −2
diff --git a/sequencer.c b/sequencer.c
index e258bb646..b0e6459a5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2948,9 +2948,9 @@ int rearrange_squash(void)
 		if (fd < 0)
 			res = error_errno(_("could not open '%s'"), todo_file);
 		else if (write(fd, buf.buf, buf.len) < 0)
-			res = error_errno(_("could not read '%s'."), todo_file);
+			res = error_errno(_("could not read '%s'"), todo_file);
 		else if (ftruncate(fd, buf.len) < 0)
-			res = error_errno(_("could not finish '%s'"),
+			res = error_errno(_("could not truncate '%s'"),
 					   todo_file);
 		close(fd);
 		strbuf_release(&buf);

-- 
2.15.0.rc0.296.g7b26d72
René Scharfe· Oct 13, 2017, 21:12 UTC · re: Ralf Thielow · lore

Re: [PATCH] sequencer.c: unify error messages

Am 13.10.2017 um 19:51 schrieb Ralf Thielow:
> When ftruncate() in rearrange_squash() fails, we write
> that we couldn't finish the operation on the todo file.
> It is more accurate to write that we couldn't truncate
> as we do in other calls of ftruncate().

Would it make sense to factor out rewriting the to-do file to avoid code duplication in the first place?

Show 17 quoted lines
> While at there, remove a full stop in another error message.
> 
> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> ---
>   sequencer.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index e258bb646..b0e6459a5 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>   		if (fd < 0)
>   			res = error_errno(_("could not open '%s'"), todo_file);
>   		else if (write(fd, buf.buf, buf.len) < 0)
> -			res = error_errno(_("could not read '%s'."), todo_file);
> +			res = error_errno(_("could not read '%s'"), todo_file);
                                                       ^^^^
That should read "write", right?
>   		else if (ftruncate(fd, buf.len) < 0)
> -			res = error_errno(_("could not finish '%s'"),
> +			res = error_errno(_("could not truncate '%s'"),
>   					   todo_file);
Hmm, why call ftruncate(2) instead of opening the file with O_TRUNC?
>   		close(fd);
>   		strbuf_release(&buf);
>
Ralf Thielow· Oct 15, 2017, 17:07 UTC · re: René Scharfe · lore

Re: [PATCH] sequencer.c: unify error messages

2017-10-13 23:12 GMT+02:00 René Scharfe <l.s.r@web.de>:
Show 29 quoted lines
> Am 13.10.2017 um 19:51 schrieb Ralf Thielow:
>> When ftruncate() in rearrange_squash() fails, we write
>> that we couldn't finish the operation on the todo file.
>> It is more accurate to write that we couldn't truncate
>> as we do in other calls of ftruncate().
>
> Would it make sense to factor out rewriting the to-do file to avoid
> code duplication in the first place?
>
>> While at there, remove a full stop in another error message.
>>
>> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
>> ---
>>   sequencer.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index e258bb646..b0e6459a5 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>>               if (fd < 0)
>>                       res = error_errno(_("could not open '%s'"), todo_file);
>>               else if (write(fd, buf.buf, buf.len) < 0)
>> -                     res = error_errno(_("could not read '%s'."), todo_file);
>> +                     res = error_errno(_("could not read '%s'"), todo_file);
>                                                        ^^^^
> That should read "write", right?
>

Sure. I'll send a new version of this patch to fix the messages. Maybe someone else picks up the other things. Thanks.

Show 10 quoted lines
>>               else if (ftruncate(fd, buf.len) < 0)
>> -                     res = error_errno(_("could not finish '%s'"),
>> +                     res = error_errno(_("could not truncate '%s'"),
>>                                          todo_file);
>
> Hmm, why call ftruncate(2) instead of opening the file with O_TRUNC?
>
>>               close(fd);
>>               strbuf_release(&buf);
>>
Johannes Schindelin· Oct 15, 2017, 15:24 UTC · re: Ralf Thielow · lore

Re: [PATCH] sequencer.c: unify error messages

Hi Ralf,
On Fri, 13 Oct 2017, Ralf Thielow wrote:
Show 6 quoted lines
> When ftruncate() in rearrange_squash() fails, we write
> that we couldn't finish the operation on the todo file.
> It is more accurate to write that we couldn't truncate
> as we do in other calls of ftruncate().
> 
> While at there, remove a full stop in another error message.
Thanks, looks good (maybe s/read/write/ as pointed out by René, though?).

Ciao, Dscho

Ralf Thielow· Oct 15, 2017, 17:07 UTC · re: Ralf Thielow · lore

[PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

When the write opertion fails, we write that we could not read. Change the error message to match the operation and remove the full stop at the end.

When ftruncate() fails, we write that we couldn't finish the operation on the todo file. It is more accurate to write that we couldn't truncate as we do in other calls of ftruncate().

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to sequencer.c +2 −2
diff --git a/sequencer.c b/sequencer.c
index e258bb646..75f5356f6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2948,9 +2948,9 @@ int rearrange_squash(void)
 		if (fd < 0)
 			res = error_errno(_("could not open '%s'"), todo_file);
 		else if (write(fd, buf.buf, buf.len) < 0)
-			res = error_errno(_("could not read '%s'."), todo_file);
+			res = error_errno(_("could not write '%s'"), todo_file);
 		else if (ftruncate(fd, buf.len) < 0)
-			res = error_errno(_("could not finish '%s'"),
+			res = error_errno(_("could not truncate '%s'"),
 					   todo_file);
 		close(fd);
 		strbuf_release(&buf);

-- 
2.15.0.rc0.296.g7b26d72
Junio C Hamano· Oct 16, 2017, 01:52 UTC · re: Ralf Thielow · lore

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

Ralf Thielow <ralf.thielow@gmail.com> writes:
Show 7 quoted lines
> When the write opertion fails, we write that we could
> not read. Change the error message to match the operation
> and remove the full stop at the end.
>
> When ftruncate() fails, we write that we couldn't finish
> the operation on the todo file. It is more accurate to write
> that we couldn't truncate as we do in other calls of ftruncate().
Wouldn't it be more accurate to say we couldn't ftruncate, though?
Show 22 quoted lines
>
> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> ---
>  sequencer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index e258bb646..75f5356f6 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>  		if (fd < 0)
>  			res = error_errno(_("could not open '%s'"), todo_file);
>  		else if (write(fd, buf.buf, buf.len) < 0)
> -			res = error_errno(_("could not read '%s'."), todo_file);
> +			res = error_errno(_("could not write '%s'"), todo_file);
>  		else if (ftruncate(fd, buf.len) < 0)
> -			res = error_errno(_("could not finish '%s'"),
> +			res = error_errno(_("could not truncate '%s'"),
>  					   todo_file);
>  		close(fd);
>  		strbuf_release(&buf);
Johannes Schindelin· Oct 16, 2017, 11:32 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

Hi Junio,
On Mon, 16 Oct 2017, Junio C Hamano wrote:
Show 11 quoted lines
> Ralf Thielow <ralf.thielow@gmail.com> writes:
> 
> > When the write opertion fails, we write that we could
> > not read. Change the error message to match the operation
> > and remove the full stop at the end.
> >
> > When ftruncate() fails, we write that we couldn't finish
> > the operation on the todo file. It is more accurate to write
> > that we couldn't truncate as we do in other calls of ftruncate().
> 
> Wouldn't it be more accurate to say we couldn't ftruncate, though?

This is an end-user facing error message, right? Should we not let users who are happily oblivious of POSIX nomenclature remain happily oblivious?

In other words, I would be finer with "truncate" than with "ftruncate... wait, huh? Is that even English?"

Ciao, Dscho

Philip Oakley· Oct 16, 2017, 12:36 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Show 20 quoted lines
> On Mon, 16 Oct 2017, Junio C Hamano wrote:
>
>> Ralf Thielow <ralf.thielow@gmail.com> writes:
>>
>> > When the write opertion fails, we write that we could
>> > not read. Change the error message to match the operation
>> > and remove the full stop at the end.
>> >
>> > When ftruncate() fails, we write that we couldn't finish
>> > the operation on the todo file. It is more accurate to write
>> > that we couldn't truncate as we do in other calls of ftruncate().
>>
>> Wouldn't it be more accurate to say we couldn't ftruncate, though?
>
> This is an end-user facing error message, right? Should we not let users
> who are happily oblivious of POSIX nomenclature remain happily oblivious?
>
> In other words, I would be finer with "truncate" than with "ftruncate...
> wait, huh? Is that even English?"
>
Hi, 'Truncate' is real English, but it is not that common in normal usage.

My dictionary suggests that it means 'cut off at the tip' such as a truncated cone. However the thesaurus is far more relaxed about the common idioms that truncate at the tail such as: clip, crop, cut short, trim, abbreviate, curtail, etc.

So perhaps "could not trim '%s'".

-- Philip

Junio C Hamano· Oct 16, 2017, 12:46 UTC · re: Philip Oakley · lore

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

"Philip Oakley" <philipoakley@iee.org> writes:
Show 8 quoted lines
> Hi, 'Truncate' is real English, but it is not that common in normal usage.
>
> My dictionary suggests that it means 'cut off at the tip' such as a
> truncated cone. However the thesaurus is far more relaxed about the
> common idioms that truncate at the tail such as: clip, crop, cut
> short, trim, abbreviate, curtail, etc.
>
> So perhaps "could not trim '%s'".

Truncate is fine, as there is already another instance that barfs with "cannot truncate" upon an error from ftruncate(), and the patch merely matches the two error messages.

← back to recent threads