lite-plugins icon indicating copy to clipboard operation
lite-plugins copied to clipboard

Initial implementation of a minimap plugin.

Open andsve opened this issue 4 years ago • 13 comments

  • Overloads DocView methods for scrollbar rendering and logic.
  • Exposes two global commands, toggling visibility of minimap and toggling syntax highlighting inside the minimap.
  • Reloading the minimap module has issues with above mentioned commands.
  • Haven't been able to test on any other platform than macOS (with a high dpi backbuffer), but using the SCALE global so should work.

image

andsve avatar May 22 '20 11:05 andsve

Very cool! Thanks for submitting this. Some initial thoughts after looking through the code and giving it a try

  • All seems to work well on my linux machine regarding the SCALE
  • I see what you mean about the per-character stuff; I didn't consider the optimisation used by the tokenizer that rolls whitespace into existing tokens would prevent only iterating the tokens
  • It seems to struggle on big files (>10k loc) as I don't believe it's culling to the currently displayed lines
  • the options at the top of the file: minimap_width etc. should be stored as values in the config table instead

A few places where performance could be improved:

  • If we avoid creating a new color table with each rectangle drawn
  • The resultant batched rectangles could be added to a weak table mapping tokens tables to tables-of-rectangles-to-draw, this means we'd typically only have to iterate the resultant rectangles for each line each frame. This would lead to a huge boost in performance

Regarding reloading the module, this isn't something that is strictly meant to be supported by plugins -- the reload module command was used more for development purposes and then changing color themes.

Also might be worth noting that the common.bench(label, function) function can be used for timing things if you're looking to make performance improvements

Additionally, as this is a larger plugin, if you're going to continue working on it would you be happy with creating a github repo for it and linking to that repo from the plugins table (similar to what the console and linter plugins do). Not a problem if you'd rather not; let me know if this is something you plan to continue expanding on, nice work so far!

rxi avatar May 22 '20 13:05 rxi

  • It seems to struggle on big files (>10k loc) as I don't believe it's culling to the currently displayed lines

Good point, planned on doing this but simply forgot, I'll fix!

  • the options at the top of the file: minimap_width etc. should be stored as values in the config table instead

Ah, of course, I fix!

  • If we avoid creating a new color table with each rectangle drawn
  • The resultant batched rectangles could be added to a weak table mapping tokens tables to tables-of-rectangles-to-draw, this means we'd typically only have to iterate the resultant rectangles for each line each frame. This would lead to a huge boost in performance

Very good point, will explore this. :+1:

Additionally, as this is a larger plugin, if you're going to continue working on it would you be happy with creating a github repo for it and linking to that repo from the plugins table (similar to what the console and linter plugins do). Not a problem if you'd rather not; let me know if this is something you plan to continue expanding on, nice work so far!

Also good point, I wasn't planning on the plugin ending up this big, but I think it's hard to do any other way. I'll do the above fixes on this branch, but move it to a separate repo once you have given a second feedback review, if you are fine with that?

Thanks for the feedback! :)

andsve avatar May 23 '20 10:05 andsve

That all sounds great! Once the culling is in, is the scrolling behaviour of the minimap something you plan to support too, eg. for cases where the document on the minimap is greater than the height of the minimap?

Regarding the rectangle caching, I was thinking something akin to this:


local cache = setmetatable({}, { __mode = "k" })

local function get_cached(tokens)
  local c = cache[tokens]
  if not c then
    c = {}
    cache[tokens] = c
    -- iterate tokens and push [x,width,color] to `c` table
  end
  return c
end

Although [x,width,color] could be stored as a table each, to save memory and possibly improve performance it might make more sense to store them flatly in the table (eg, push first the x-offset, then width then color, then the next x-offset etc. and then iterate the table with a step of 3)

Since we're using a weak table we never have to worry about cleaning things up, and since we're using the tokens tables as keys, which are never reused, we don't have to worry about invalidating the cache.

Otherwise great work -- glad to hear you're going to continue working on it!

rxi avatar May 23 '20 12:05 rxi

Just wanted to give you a quick update!

That all sounds great! Once the culling is in, is the scrolling behaviour of the minimap something you plan to support too, eg. for cases where the document on the minimap is greater than the height of the minimap?

This is what I have been fixing today, seem to work much better with larger files already. (Added culling as well.)

large_scroll2 Maybe a bit hard to see in this compressed gif, but this is scrolling a ~10k lines C file, tried to keep the behaviour close to what Sublime does at least.

Regarding the rectangle caching, ... I think that sounds good, haven't had time to look at better caching yet but this will come in handy. This is next on my list! :+1:

andsve avatar May 24 '20 18:05 andsve

Looks great! Looking forward to seeing how it performs with the rectangle caching too, but otherwise it seems like it's basically there. Really nice work.

Did you plan on adding a background to it? I wasn't sure if this was something that just wasn't a priority yet or if you were going for a different aesthetic.

Thanks for the update!

rxi avatar May 25 '20 07:05 rxi

Hope you dont mind me chiming in, is the height adjustable? I would prefer being able to set a max height.

Anachron avatar May 25 '20 11:05 Anachron

Did you plan on adding a background to it? I wasn't sure if this was something that just wasn't a priority yet or if you were going for a different aesthetic.

I'll add a background rect, but also an option to disable it. :+1:

Hope you dont mind me chiming in, is the height adjustable? I would prefer being able to set a max height.

Should be possible to add this as a option, do you just mean that the minimap should only be visible up to a certain px height? Do you want a scrollbar along with it? Currently this replaces the scrollbar.

andsve avatar May 25 '20 11:05 andsve

Yes I would still want the scrollbar. The minimap only in the top X pixels. This way my view will stay sleek while still giving me some overview of the file.

Anachron avatar May 25 '20 13:05 Anachron

@andsve two small things I noticed:

  • With how the override on on_mouse_moved is done — where the original function isn't called — that other plugins that patch into the same function stop working, for example this breaks the linter plugin
  • How the background color is set means that changing the theme doesn't update the background color. In other plugins that use fall-back colors I'd do: style.minimap_background_color or style.background_color at the usage point (the color would be expected in the style table too)

rxi avatar May 26 '20 18:05 rxi

  • With how the override on on_mouse_moved is done — where the original function isn't called — that other plugins that patch into the same function stop working, for example this breaks the linter plugin
  • How the background color is set means that changing the theme doesn't update the background color. In other plugins that use fall-back colors I'd do: style.minimap_background_color or style.background_color at the usage point (the color would be expected in the style table too)

Good points, pushed a change that hopefully fixes this.

Haven't had time to look into caching due to work, but hopefully within a day or two.

edit: Checked with the linters plugin (specifically the one using luacheck), seems to work fine. image

andsve avatar May 27 '20 19:05 andsve

Works well for me. Is there a chance of adding this plugin to the list of preinstalled ones? It would be cool if all themes supported the background color of the minmap.

Jipok avatar Nov 03 '20 20:11 Jipok

With the minimap, when I open the command window, such a bar appears: image

Also, the minimap does not drag with the mouse if you click and drag without releasing the left button: tmp

Jipok avatar Nov 04 '20 12:11 Jipok

For long lines I can't get to their very end. This is either using the 'end' key or trying to select a line up to its end:

https://user-images.githubusercontent.com/12665563/106764321-faafed00-6637-11eb-8a4a-3c58973a8600.mp4

terencode avatar Feb 03 '21 14:02 terencode