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

[Yuan Fu <casouri@gmail.com>]

> 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