Re: [PATCH 1/3] scripts: add spelling_sanitizer.sh script

[Leizhen (ThunderTown)]

On 2021/6/11 15:58, Andy Shevchenko wrote:
> On Fri, Jun 11, 2021 at 10:19 AM Zhen Lei <thunder.leizhen@xxxxxxxxxx> wrote:
>>
>> The file scripts/spelling.txt recorded a large number of
>> "mistake||correction" pairs. These entries are currently maintained in
>> order, but the results are not strict. In addition, when someone wants to
>> add some new pairs, he either sort them manually or write a script, which
>> is clearly a waste of labor. So add this script. It removes the duplicates
>> first, then sort by correctly spelled words. Sorting based on misspelled
>> words is not chose because it is uncontrollable.
> 
> chosen

OK

> 
> ...
> 
>> +#!/bin/sh
> 
> If you want to have stricter rules applied, use
> #!/bin/sh -efu
> in all your shell scripts, it will show you a lot of problems.
> 
> Missed SPDX.

OK, I will add it.

> 
>> +src=spelling.txt
> 
>> +tmp=spelling_mistake_correction_pairs.txt
> 
> It will pollute the source tree, so use `mktemp` or utilize O=. In
> case there is no O= supplied (or whatever equivalent to describe
> output folder) you will get it in the source tree, so it needs to be
> Git-ignored.

OK, I will use mktemp to generate the tmp file.

> 
>> +cd `dirname $0`
> 
> Useless use of dirname. Check for %, %%, #, and ## substitutions (`man sh`).
> IIRC dirname equivalent is ${0%/*}.

I just tried it. It works.

> 
>> +# Convert the format of 'codespell' to the current
>> +sed -r -i 's/ ==> /||/' $src
>> +
>> +# Move the spelling "mistake||correction" pairs into file $tmp
> 
>> +# There are currently 9 lines of comments in $src, so the text starts at line 10
>> +sed -n '10,$p' $src > $tmp
>> +sed -i '10,$d' $src
> 
> This is fragile, use proper comment line detection.

I've thought about that too. But I'm wondering if it needs to be that
complicated.

Think about it. It's not something for personal temporary use, so it
should be perfect. I'll change to dynamic computing.

> 
>> +# Remove duplicates first, then sort by correctly spelled words
>> +sort -u $tmp -o $tmp
>> +sort -t '|' -k 3 $tmp -o $tmp
> 
> Can be one pipeline

OK, I will combine it.

> 
>> +# Append sorted results to comments
>> +cat $tmp >> $src
> 
> I believe it can be done in a better way, but I was not thinking about it.

I'll keep searching.

> 
>> +# Delete the temporary file
>> +rm -f $tmp
> 
> What if the script will be trapped? It's good to handle SIGHUP I
> suppose, so we won't leave garbage behind us.

You're very well thought out. I'll take care of it.

> 
>> +cd - > /dev/null
>