lush.nvim icon indicating copy to clipboard operation
lush.nvim copied to clipboard

Improving `:Lushify` with tree-sitter

Open savq opened this issue 2 years ago • 10 comments

Ok, this is either dumb or genious, but bear with me.

Right now, :Lushify uses regexes to highlight hsl calls and group names. That's all well and good, except it only highlights very limited patterns. It can't highlight hsl calls that use variables as arguments, or color method calls, (da, li etc).

I like looking at all the colors I'm using, so I write all hsl calls with literals instead of variables. Even though I use a small set of hsl values, I'm defining a lot of colors so there's a lot of repetition. Once I move the colorscheme to hsluv (nice addition btw) I'll probably have more redundant definitions, since I'll need to tweak things less to make the colors look uniform.

Now, we kinda already have a Lua parser lying around. We could use tree-sitter to parse the spec, and highlight more patterns without needing to define regexes for each one. :Lushify already evaluates the buffer, so we can use that to get the values of variables used in hsl calls (I think?).

This may or may not need the treesitter highlighting module. Getting the text regions from the treesitter nodes should be enough.

I'd be interested in adding this, though the docs for the treesitter modules are always… a work in progress, so it might take a while. I'd like to know if this actually fits the project before jumping in.

savq avatar Jul 13 '21 20:07 savq

Yeah this is something I would like to fix, I have sort of thought about it off and on, here's a bit of a brain dump:

There were 3 main choices in my head:

  • canibalize the lua state when we run loadstring() on the buffer
    • pretty ugly solution if it's even possible
  • Treesitter
    • lightest?
  • LSP
    • Would allow for better error reporting too
    • ideally runs in-app as a separate thread

I have briefly played with Treesitter (writing some parts of a parser for elixir, and using an existing parser as a transform step https://github.com/rktjmp/html-to-temple).

Writing a whole new parser specifically for lush-specs seemed like a real can of worms so I didn't follow that (I know you're not suggesting that exactly).

From memory when working with the HTML parser, I could get the node details such as it's name, associated attributes, but I don't know if you can use the tree to find "deep" values, at least not for free, but I also wasn't trying to do that.

local red = hsl(h_val, s_val, l_val)

I think the parser will tell you something like:

((variable_dec
  (left (identifier))
  (right
    (function_call 
      (identifier)
      (argument (identifier))
      (argument (identifier))
      (argument (identifier))))

You should be able to extract the identifiers names, then I guess query the tree for any variable matching that identifier name and value, push that into memory and use it to build the HSL call?

Some considerations:

  • tree sitter is currently "slow"?
    • I've seen a few posts about this and the release notes warn about it. I don't really know how slow though. I think it's mostly limited to multi-language files? I only bring it up because it's part of the real-time system so if TS is likely to add a lot of latency I am not sure it's a good fit, yet.
    • It wont be "slow" at some point though so still fun to look into now.
  • dependencies
    • installing something to make/use colorschemes is already a bit weird, so I would want any solution to be bundled in the main repo unless it's unwieldy, no pac { 'lush/lush-treesitter' } if possible.
    • would need to maintain basic regex highlighter for people who dont install TS or TS-Lua (if we use that).
  • moving API
    • not sure how stable nvim-ts's API is, wouldn't want to have to be fixing stuff all the time as it shifts?
  • TS is only "per file", so imports get lost, you can infer if they are hsl's by usage in some cases.

I might recommend getting the lua treesitter project and building the web playground for it. It might be faster to hack on when writing queries and exploring stuff (console.log is generally more inspectable than vim.inspect)?

Anyway, that's a lot of text.

  • Yes I want this better.
  • TS is worth exploring.
  • Some minimal POC for us to look at would be helpful? Probably needs to:
    • identify hsl/hsluv calls (just hsl/hsluv is ok for now, don't need to track mk_clr = lush.hsl and then highlight mk_clr(...).
    • track identifiers used as args (hsl(tracked_val, 20, 20))
    • track hsl vars (local track_me = hsl(...))
    • does not need to integrate with lush atm, just "parse a spec as lua then introspect" to see if we can apply it practically.

rktjmp avatar Jul 14 '21 03:07 rktjmp

Here's a really quick playground https://github.com/rktjmp/treesitter-lua-webplayground, should just be a

npm install
npx vite

and edit main.js#translate

Just thinking out loud about variable tracking, not sure if TS will track actual variables, or just tell you their name (believe it's just names), which might make something like this break:

local val = 10
local c = hsl(val, 44, 55)
val =  "all done"

We would have to be sure to parse top down, or only ever look up values which match the appropriate state for each line. I remember there were a few issues like this when I was thinking about it months ago. I think that is what pushed me towards the LSP route since the state tracking had to be more complex.

This needs to work:

local val = 10
local c = hsl(val, 44, 55)
local cs = c.sa(10)
val =  "all done"
val = 99
c = hsl(val, 10, 10)
local cl = c.li(20)

rktjmp avatar Jul 14 '21 04:07 rktjmp

You should be able to extract the identifiers names, then I guess query the tree for any variable matching that identifier name and value, push that into memory and use it to build the HSL call?

Yes, that's roughly what I had in mind.

tree sitter is currently "slow"?

That depends on the language really. Languages with really complicated bolted-on syntax (C++, TS, etc) seem to be the worst. Lua doesn't feel slow at all.

dependencies

I thought the Lua parser was included in core, but only the C parser is included. so yeah, this would require the nvim-treesitter plugin :(

moving API

The query syntax isn't changing afaik, and that's the most important part here.

I might recommend getting the lua treesitter project and building the web playground for it. It might be faster to hack on when writing queries and exploring stuff (console.log is generally more inspectable than vim.inspect)?

I'm using nvim-treesitter/playground and it's working great.


Just thinking out loud about variable tracking, not sure if TS will track actual variables, or just tell you their name

Just the name. Although I feel like this wouldn't be an issue in practice. Most Lush colorschemes are written in a very declarative style.

savq avatar Jul 14 '21 22:07 savq

Writing a query like

((function_call (identifier) @lushcall
  (arguments (_) @lushcolor)
  (eq? @lushcall "hsl"))) @lushcolor

in some_plugin/after/queries/lua/highlights.scm, and then setting the lushcolor highlight would correctly highlight an hsl call and its arguments (although not the parentheses and commas), without highlighting other function calls.

:h lua-treesitter-directives mentions something about setting metadata attributes. I think this could be used to store the names/number-values of the arguments. although I'm not sure about how one actually accesses these attributes from Lua.

EDIT: The query doesn't handle binary expressions like hsl(x+1, x+2, x+3). It seems other queries take more precedence in that case, even if the query was defined in after/.

savq avatar Jul 14 '21 22:07 savq

You'll need to do more introspection than just the query. (just rough untested code, esp towards the end)

  local parser = vim.treesitter.get_parser(buf, "lua")
  -- parse() actually returns trees, not tree
  local trees, changes = parser:parse()
  local tree = trees[1]

  -- iter_captures returns all captures, *probably* in order, but that's not
  -- documented
  --
  -- query = vim.treesitter.parse_query("lua", [[
  --   (function_call
  --     (identifier) @fn_name
  --     (arguments (_) @fn_arg)
  --     (any-of? @fn_name "hsl" "hsluv")) @call
  -- ]])
  --
  -- query:iter_captures(...) -> [
  --   function_call, identifier (hsl), string,
  --   functon_call, ...
  -- ]
  --
  -- since it's undocumented, it feels it may be better to inspect with
  -- separate queries for each part.

  -- find any hsl(uv) calls
  local query_calls = vim.treesitter.parse_query("lua", [[
    (function_call
      (identifier) @fn_name
      (arguments (_))
      (any-of? @fn_name "hsl" "hsluv"))
  ]])

  local query_call_string = vim.treesitter.parse_query("lua", [[
    (function_call
      (identifier) @fn_name
      (arguments (string) @fn_arg)
      (any-of? @fn_name "hsl" "hsluv")) @call
  ]])

  for _, node in query_calls:iter_captures(tree:root(), buf) do
    local fn_type = vim.treesitter.get_node_text(node, buf)
    local fn_call = vim.treesitter.get_node_text(node:parent(), buf)
    -- could also find arguments child, inspect type for "string", get that nodes text
    -- print(fn_type, fn_call)

    -- try to create hsl color
    local make_hsl = function(type, args)
      -- somehow handle args that might be:
      --  "#arst"
      --  "1, 2, 3"
      --  "my_h(), 10, 20"
      -- maybe just
      -- local doit = loadstring("require('lush')." .. __type__ .. "(".. args .. ")")
      -- local s, c = pcall(doit)
      -- may not need require since load* runs in the same lua state as the
      -- caller.
      -- if s return c else nil 
    end
    local c = make_hsl(fn_type, fn_call)

    -- apply color to call range
    local rs, cs, re, ce = node:range()
    local group = hl_group_for_hsl(fn_type.."-"..c.to_string(), c) -- name, bg color
    api.nvim_buf_add_highlight(buf, hl_group_ns, group, rs, cs - 1, ce)
  end

  -- not totally clear if this is more useful iter
  --  for pattern, match, metadata in query_calls:iter_matches(tree:root(), buf) do
  --    print(vim.inspect(pattern), vim.inspect(match))
  --    for id, node in pairs(match) do
  --      local fn_call = vim.treesitter.get_node_text(node, buf)
  --      print(fn_call)
  --    end
  --  end

My take away after poking around is that you could convert basic calls with reasonable effort, (basic being hsl(string) or hsl(number, number, number)), but it's still not much of an aid to highlight anything like:

    Normal       { bg = colors.ocean_500, fg = colors.crest_700 },
                        ^^^^^^^^^^^^^^^^       ^^^^^^^^^^^^^^^^

If it were included by default, the TS query might be preferable to match basic calls if only because it's cleaner code than regex but since it's an separate install you'd just have to maintain both versions anyway, so not much to gain.

Matching anything that takes a variable seems like it would be a bit of a nightmare, not impossible but pretty awkward. Lua has a debug lib, http://www.lua.org/manual/5.1/manual.html#5.9, but it might have negative performance impact, and still won't give you "line values", if a variable changes.

I end up applying my operators when I set a group anyway, so highlighting them ends up being not so useful:

   Group { fg = col.red.da(30).de(20) }
-- ^^^^^ shows end result anyway  

And since groups can use other groups, I sometimes just do this to see the changes to colors:

altered { bg = col.red.da(30).de(20) }
Group { fg = altered.bg }

I think TS is a good fit to apply chained highlights though:

local red = hsl(10, 20, 20).de(20)
            ^^^^^^^^^^^^^^^^^^^^^^ include desat effect in highlight

Possibly highlight red and any other usage of a variable with red as it's name, still hits a "change value" issue though.

rktjmp avatar Jul 15 '21 03:07 rktjmp

Not exactly what you started the thread about but an improvement:

Peek 2021-07-15 16-56

Gif seemed to record in a mega slow framerate for some reason.

https://github.com/rktjmp/lush.nvim/compare/main..feat-ts

rktjmp avatar Jul 15 '21 06:07 rktjmp

The script looks pretty good.

[...] since it's an separate install you'd just have to maintain both versions anyway, so not much to gain.

Yes, this is the thing that bums me out. I don't know why I was so sure that the lua parser was in core… Although it's worth considering that only colorscheme developers need it, not the users.

And since groups can use other groups, I sometimes just do this to see the changes to colors

That's a pretty good idea. I'll see if I can use it.

I think TS is a good fit to apply chained highlights though

Yes, if it's possible to do this with variables, then I'd be great, because one could do color.ro(0) to get a highlight. Not as nice as getting the variable highlighted on it's own, but much simpler to implement.

savq avatar Jul 17 '21 00:07 savq

Tracking variables feels like it could be a maintenance pain, since we're basically writing less tested, worse, lua language parser. There are just so many edges around imports, renaming, bad values, nested tables, etc, etc.

I am wondering if just having a more structured method for defining colors optionally might be the way to do. Not really sure how it would work, maybe:

local palette = lush.palette({
  dark = {...}
  ...
})

and we can somehow track that table specifically when lushify is running.

rktjmp avatar Jul 17 '21 00:07 rktjmp

Tracking variables feels like it could be a maintenance pain

Yeah, I probably ignored too many edge cases when I first thought about this feature.

I am wondering if just having a more structured method for defining colors optionally might be the way to do

I'm not sure about it either. Since colors defined in a table couldn't reference other colors in the same table.

savq avatar Jul 17 '21 22:07 savq

It would probably end up being similar to the actual group code, where it's kind of build post-facto. Even then it probably hits similar issues when applying highlights reliably.

I don't think it's a great solution because it adds another API layer to learn when using Lush. If I did something like that it would remain optional, basically only needed if you wanted "aware highlighting".

rktjmp avatar Jul 18 '21 00:07 rktjmp