Re: [PATCH] amend error message violations

From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Date: Oct 25, 2021, 16:40 UTC
Message-ID: <d9edec0c-434f-800d-42ea-222423e7da04@gmail.com>
In-Reply-To: <211025.86y26hgtm0.gmgdl@evledraar.gmail.com>

On 25-10-2021 18:07, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> On Mon, Oct 25 2021, Abhradeep Chakraborty via GitGitGadget wrote:
>
>> From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
>>
>> Issue no #635 in the gitgitgadget/git repo has a brief description
>> about the violation of error message style convention. These
>> violations are - 1) Do not end error messages with a full stop. 2) Do
>> not capitalize ("unable to open %s", not "Unable to open %s") 3) Say
>> what the error is first ("cannot open %s", not "%s: cannot open")
> Let's summarize the guidelines instead of asking readers to visit
> https://github.com/gitgitgadget/git/issues/635 (which isn't even
> directly linked here) only to find a quote from CodingGuidelines.

sorry for not mentioning that. I didn't realize at the time of writing

the commit message.

> I think it's good to split these up, e.g. the rewording of the BUG()
> message for the 3rd quoted item in CodingGuidelines from cases where
> you're only changing the capitalization of these messages.

But both of these don't comply with the error message style convention.

So, don't you think that all should be in the same commit (all of them

are equally violating style conventions)?

>> If we're going to mass-edit these I wouldn't mind an addition to the
>> guidelines that says we ''-quote things, and add that when appropriate....

Pardon me, I didn't understand the "we ''-quote things, and add that when appropriate" part. Could you please explain that again?

Show 9 quoted lines

>> -	error(_("You have not concluded your merge (MERGE_HEAD exists)."));
>> +	error(_("you have not concluded your merge (MERGE_HEAD exists)"));
>>   	if (advice_enabled(ADVICE_RESOLVE_CONFLICT))
>>   		advise(_("Please, commit your changes before merging."));
>> -	die(_("Exiting because of unfinished merge."));
>> +	die(_("exiting because of unfinished merge"));
>>   }
> Surely any convention of not using full-stop should extend to advise()
> too?

I was thinking of it and even was confused at the starting. But later I thought

that advise and errors are not the same thing. So, there shouldn't be any change

for advise(). Though I am not so confident about this.

> But in this case aren't we constructing sentences in parts? Maybe having
> the full-stops here is better than not, I'm not sure.

Maybe you're right. One question to ask is, do error() and advise()

add "\n" at the end of these strings?

Show 7 quoted lines

> Also, have you tried using coccinelle for this? Doing this sort of
> replacement should be a small matter of some inline Python, the
> linux.git repo has some good examples of that.
>
> It would make discussing these changes easier, and per the "pending"
> docs in contrib/coccinelle/README any patches could follow such rules
> incrementally...

No, I haven't used coccinelle but will definitely take a look at it.

Thanks.