Re: Initial fontification in sh-mode with tree-sittter

[Yuan Fu]

> On Nov 17, 2022, at 10:53 AM, João Paulo Labegalini de Carvalho 
> <jaopaulolc@gmail.com> wrote:
> 
>  
> 1. I added commit message for your patch as an example. For future patches 
> it’s best if you can also have this ChangeLog style commit message, I think 
> you can find more detailed explanation in in CONTRIBUTE file.
> 
> (I don’t know if you know the following already, bug FYI:)
> 
> 2. Docstring sentences always end with a period
> 3. All comments and sentences should be complete sentences, with two spaces 
> at the end.
> 4. The first line of a docstring shouldn’t exceeds 80 columns.
> 
> I did not. Thanks for the corrections and improvements. I will keep those in 
> mind for future patches. 
> 
> For the second commit, I changed all the feature names to singular, and 
> decl-commands to declaration-command. I also simplified the rule for 
> declaration-command, IIUC you want to highlight the command keywords?
> 
> The changes to singular make sense to me. I considered your simplified 
> version as well, but decided against alternation queries to avoid hardcoded 
> command names. Matching a node via its type rather than comparing the 
> spelling might be fast, but my code also had a string= there, so in the end 
> the simpler version works well.

It should be safe to hardcode these command keywords, because they are 
hardcoded in the grammar anyway (and I don’t see them used elsewhere):

    declaration_command: $ => prec.left(seq(
      choice('declare', 'typeset', 'export', 'readonly', 'local'),
      repeat(choice(
        $._literal,
        $._simple_variable_name,
        $.variable_assignment
      ))
    )),

> 
> Also, right now these command keywords are highlighted in builtin face, 
> should they be fontified in keyword face? I’m no expert of bash so I can’t 
> tell. But keyword face seems more natural to me.
> 
> You are correct in the sense that all declaration commands are builtin 
> commands. I decided to highlight them differently than other builtin commands 
> because they are used to define variables, and it might be useful to be able 
> to distinguish them visually. But this is not required nor more correct than 
> using the builtin face for both types of commands. 

I meant to highlight them all in keyword face, but if you don’t have objections 
I’ll make that change :-) Thanks!

Yuan