# [PATCH] sequencer.c: unify error messages 9 messages from 2017-10-13 to 2017-10-16. Participants: Ralf Thielow, René Scharfe, Johannes Schindelin, Junio C Hamano, Philip Oakley. Thread: https://gitlist.dev/t/46966 ## Ralf Thielow, 2017-10-13 17:51 Subject: [PATCH] sequencer.c: unify error messages Message-ID: <20171013175157.13634-1-ralf.thielow@gmail.com> URL: https://gitlist.dev/e/20171013175157.13634-1-ralf.thielow%40gmail.com ``` 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 --- 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); 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, 2017-10-13 21:12 Subject: Re: [PATCH] sequencer.c: unify error messages Message-ID: <58336f0d-366c-6636-1e94-22afbafdfe4c@web.de> URL: https://gitlist.dev/e/58336f0d-366c-6636-1e94-22afbafdfe4c%40web.de In-Reply-To: <20171013175157.13634-1-ralf.thielow@gmail.com> ``` 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 > --- > 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); > ``` ## Johannes Schindelin, 2017-10-15 15:24 Subject: Re: [PATCH] sequencer.c: unify error messages Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.21.1.1710151724200.40514%40virtualbox In-Reply-To: <20171013175157.13634-1-ralf.thielow@gmail.com> ``` Hi Ralf, On Fri, 13 Oct 2017, Ralf Thielow wrote: > 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, 2017-10-15 17:07 Subject: Re: [PATCH] sequencer.c: unify error messages Message-ID: URL: https://gitlist.dev/e/CAN0XMOKjhoi-_bWqOpF-uTtaF8YrmPfR_0RMDePZSzOxAi2bwQ%40mail.gmail.com In-Reply-To: <58336f0d-366c-6636-1e94-22afbafdfe4c@web.de> ``` 2017-10-13 23:12 GMT+02:00 René Scharfe : > 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 >> --- >> 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. >> 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, 2017-10-15 17:07 Subject: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash() Message-ID: <20171015170742.8395-1-ralf.thielow@gmail.com> URL: https://gitlist.dev/e/20171015170742.8395-1-ralf.thielow%40gmail.com In-Reply-To: <20171013175157.13634-1-ralf.thielow@gmail.com> ``` 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 --- 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); -- 2.15.0.rc0.296.g7b26d72 ``` ## Junio C Hamano, 2017-10-16 01:52 Subject: Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash() Message-ID: URL: https://gitlist.dev/e/xmqqfuajuae8.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20171015170742.8395-1-ralf.thielow@gmail.com> ``` Ralf Thielow 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? > > Signed-off-by: Ralf Thielow > --- > 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, 2017-10-16 11:32 Subject: Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash() Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.21.1.1710161331290.40514%40virtualbox In-Reply-To: ``` Hi Junio, On Mon, 16 Oct 2017, Junio C Hamano wrote: > Ralf Thielow 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, 2017-10-16 12:36 Subject: Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash() Message-ID: <17346CCF8DEF41E790C96248408E07D2@PhilipOakley> URL: https://gitlist.dev/e/17346CCF8DEF41E790C96248408E07D2%40PhilipOakley In-Reply-To: ``` From: "Johannes Schindelin" > On Mon, 16 Oct 2017, Junio C Hamano wrote: > >> Ralf Thielow 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, 2017-10-16 12:46 Subject: Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash() Message-ID: URL: https://gitlist.dev/e/xmqqwp3vqmz4.fsf%40gitster.mtv.corp.google.com In-Reply-To: <17346CCF8DEF41E790C96248408E07D2@PhilipOakley> ``` "Philip Oakley" writes: > 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. ```