rktjmp / lush.nvim

Create Neovim themes with real-time feedback, export anywhere.
MIT License
1.44k stars 47 forks source link

Improving `:Lushify` with tree-sitter #52

Closed savq closed 11 months ago

savq commented 3 years ago

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.

rktjmp commented 3 years ago

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:

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:

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.

rktjmp commented 3 years ago

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)
savq commented 3 years ago

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 commented 3 years ago

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/.

rktjmp commented 3 years ago

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 commented 3 years ago

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

savq commented 3 years ago

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.

rktjmp commented 3 years ago

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.

savq commented 3 years ago

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.

rktjmp commented 3 years ago

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".