Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Direct access to cell styling via API #1180

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Conversation

dther
Copy link
Contributor

@dther dther commented Apr 21, 2024

This pull request essentially exposes the new style handling from PR #1154 to the Lua API, allowing plugins to make direct changes to cells using a new Win method, Win.style_pos.

It also adds a new style constant, STYLE_LEXER_MAX, which provides a reliable way of defining new user styles without needing to interact with the lexer.

Finally, a new event in ui-terminal.c, UI_DRAW, allows plugin developers the opportunity to override all previous styling. This allows for the creation of new UI elements entirely within the Lua API. This event is emitted at quite a low level (outside of vis.c and in ui_draw, right before the backend blit function), so I'd like opinions on whether it's even a good idea to have events outside of vis.c. In my opinion, it's a reasonably UI-specific event which has different ramifications based on the GUI toolkit being used, so aside from perhaps an extra argument to indicate more information to the user (i.e. "terminal/curses/ansi", "tk", "gnome", w/e). I don't foresee many changes to it.

By exposing a low-level styling function, many feature requests become implementable using the Lua API alone. For example, the following snippet can be placed into visrc.lua in order to implement status bar styling (#1128):

-- Example usage for style_pos: styling the status bar
local modes = {
	[vis.modes.NORMAL] = 'NORMAL',
	[vis.modes.OPERATOR_PENDING] = '???',
	[vis.modes.VISUAL] = 'VISUAL',
	[vis.modes.VISUAL_LINE] = 'VISUAL-LINE',
	[vis.modes.INSERT] = 'INSERT',
	[vis.modes.REPLACE] = 'REPLACE',
}

vis.events.subscribe(vis.events.WIN_STATUS, function(win)
	local left_parts = {}
	local right_parts = {}
	local file = win.file
	local selection = win.selection

	local mode = modes[vis.mode]
	if mode ~= '' and vis.win == win then
		table.insert(left_parts, mode)
	end
-- insert the rest of the code from vis-std.lua here...
	local left = ' ' .. table.concat(left_parts, " » ") .. ' '
	local right = ' ' .. table.concat(right_parts, " « ") .. ' '
	win:status(left, right);

	-- this must be done after calling win:status()
	-- mode change styling
	win.STYLE_RED = win.STYLE_LEXER_MAX - 1
	win.STYLE_GREEN = win.STYLE_LEXER_MAX - 2
	win.STYLE_YELLOW = win.STYLE_LEXER_MAX - 3
	win.STYLE_BLUE = win.STYLE_LEXER_MAX - 4
	win.STYLE_CYAN = win.STYLE_LEXER_MAX - 5
	win.STYLE_MAGENTA = win.STYLE_LEXER_MAX - 6
	win:style_define(win.STYLE_RED, "fore:red,reverse,bold")
	win:style_define(win.STYLE_GREEN, "fore:green,reverse,bold")
	win:style_define(win.STYLE_YELLOW, "fore:yellow,reverse,bold")
	win:style_define(win.STYLE_BLUE, "fore:blue,reverse,bold")
	win:style_define(win.STYLE_CYAN, "fore:cyan,reverse,bold")
	win:style_define(win.STYLE_MAGENTA, "fore:magenta,reverse,bold")

	local modestyle = {
		[vis.modes.NORMAL] = win.STYLE_BLUE,
		[vis.modes.OPERATOR_PENDING] = win.STYLE_YELLOW,
		[vis.modes.VISUAL] = win.STYLE_GREEN,
		[vis.modes.VISUAL_LINE] = win.STYLE_CYAN,
		[vis.modes.INSERT] = win.STYLE_RED,
		[vis.modes.REPLACE] = win.STYLE_MAGENTA,
	}

	local style = modestyle[vis.mode]
	if style and vis.win == win then
		for c=0,#modes[vis.mode]+1 do
			win:style_pos(style, c, win.height - 1)
		end
	end
end)

And UI_DRAW can be used, for example, to allow tighter integration with debugging tools by visually marking important lines:

-- UI_DRAW example usecase: highlighting important line numbers
-- using scraped info from cc or gdb or whatever
local importantfile = "vis.c"
local linesinview = {3, 23, 6, 15}

vis.events.subscribe(vis.events.UI_DRAW, function()
	local win = vis.win
	if not win then
		return
	end
	local file = win.file.name
	if not file or file ~= importantfile then
		return
	end
	if not win.STYLE_RED then
		return
	end

	-- highlight the line number and the rightmost column
	for line=1,#linesinview do
		if win.options.numbers then
			win:style_pos(win.STYLE_RED, 0, linesinview[line] - 1)
			win:style_pos(win.STYLE_RED, 1, linesinview[line] - 1)
		end
		win:style_pos(win.STYLE_RED, win.width - 1, linesinview[line] - 1)
	end
end)

And the end result:
image

@rnpnr
Copy link
Collaborator

rnpnr commented Apr 21, 2024

Very interesting! I definitely like this more than the line numbers styling patch from the wiki. This obviously has some footguns like the WIN_HIGHLIGHT event but I don't really think that's a problem.

I'm going to cherry pick the first two commits since they are unrelated, and leave you to fix the compilation issues. I also want to hear what other people think!

@dther
Copy link
Contributor Author

dther commented Apr 22, 2024

Thanks for checking! It seems I missed a stub function in vis-lua.c. Should be fine now, I've tested it on my system with ./configure --disable-lua && make clean vis.

@fischerling
Copy link
Contributor

Very interesting! I definitely like this more than the line numbers styling patch from the wiki. This obviously has some footguns like the WIN_HIGHLIGHT event but I don't really think that's a problem.

I also want to hear what other people think!

I really like the simplicity and universal approach of this API.

However, the line numbers patch has the advantage that it allows to easily style lines not currently visible in the viewport.

In LSP land the server provides the information that in the file at line 200 there is a certain diagnostic.
vis-lspc can simply tell vis to highlight the line 200, and it will be styled if visible.
With this API vis-lspc needs to map the line number in the file to the viewport.

If anyone has a good idea how to efficiently map file line numbers to viewport line number I would gladly support this API in vis-lspc.

Maybe exporting the line range of the viewport to Lua could be a simple solution?

@dther
Copy link
Contributor Author

dther commented Apr 23, 2024

If anyone has a good idea how to efficiently map file line numbers to viewport line number I would gladly support this API in vis-lspc.

This is something I've wanted for a long time too. It would greatly simplify a lot of the code I've written for the experimental mouse support, which relies heavily on translating visual lines to absolute byte position. Granted, I now plan to implement that logic directly in C instead, but I'm glad to see that I'm not the only one who sees value in exposing this data to the Lua API.

Maybe exporting the line range of the viewport to Lua could be a simple solution?

I'm thinking of something along these lines as well. My initial thoughts are to output a simple table/list indexed by viewport lines, with member variables being the actual logical file lines, and perhaps special keys for "first" and "last", since this is what's exposed in view.h. In the future this might lead to a Lua class that exposes struct View with more power, but in my opinion it's best we keep things simple for the first prototype.

@rnpnr Should I add the commits for this proposed feature onto this PR? On the one hand, it's a function that's primarily useful due to the new UI_DRAW event, but on the other, I'd like this PR to be merged sooner rather than later, so that others can experiment with what's already possible.

Edit: On the topic of expanding the Lua API's access to UI layout, there's a lot of information in ui-terminal.h that would be useful for properly styling parts of the viewport. sidebar comes to mind, as it's a simple integer that can be used to figure out how many cells to style in order to highlight an entire line number.

@rnpnr
Copy link
Collaborator

rnpnr commented Apr 23, 2024

My initial thoughts are to output a simple table/list indexed by viewport lines, with member variables being the actual logical file lines

I think we would want something simpler than that, for example just a Range with the first and last line in view. We already have File.lines in lua which lets you get the lines contents by line number. On the C side the first and last line are already easily accessible via win->view.

Should I add the commits for this proposed feature onto this PR?

It should be a separate submission. From a brief look at what is already available I don't really expect such a feature to need more than ~10 lines of code.

@dther
Copy link
Contributor Author

dther commented Apr 23, 2024

I think we would want something simpler than that, for example just a Range with the first and last line in view. We already have File.lines in lua which lets you get the lines contents by line number. On the C side the first and last line are already easily accessible via win->view.

Apologies for the confusion- when I mean mapping visual lines to logical lines, I meant by line number-to-line number, not content. While simply returning the first and last line numbers is much simpler than what I've implemented, I didn't have much trouble, and still believe that, exposing lineno values stored in win->view->lines should make for much simpler Lua code. I still shudder when I think about how large guess_file_pos (my lua function for translating terminal x, y coordinates to file lines and columns) ended up being. See #1181.

@rnpnr
Copy link
Collaborator

rnpnr commented Apr 24, 2024

Sorry, I guess I forgot that the viewport height and width isn't visible from lua right now, only the height/width of windows. Personally I would be ok if those were exposed separately.

Continuing this train of thought, I think long term the useful dimensions are the full UI width/height and the View width/height. With these two you can determine the sidebar width and status line height (if for example the info window is open). The window width and height which includes the sidebar and status line are only useful now while we allow separate windows.

@dther
Copy link
Contributor Author

dther commented Apr 25, 2024

As requested, I've moved the code for view height and width from #1181 over here. I imagine it would be best to make them fields within the new viewport table, so I plan to do that if no one objects- but I'll hold off on committing until #1181 is merged so that I can fast-forward cleanly.

@dther
Copy link
Contributor Author

dther commented Apr 25, 2024

Well, so much for a clean fast-forward. I'll rebase and force-push later. (unless anyone objects between now and then)

@rnpnr
Copy link
Collaborator

rnpnr commented Apr 25, 2024

Nope, that is actually preferred. Github doesn't really play nice with it but everyone maintaining vis is used to the patch file/linear history approach.

@fischerling
Copy link
Contributor

fischerling commented Apr 25, 2024

Thank you both for your work :)

Wanted to leave the function I am going to use in vis-lspc to map file line number to viewport line number, combining both #1181 and this.

local function file_lineno_to_viewport_lineno(win, file_lineno)
  -- The line is not in the current viewport
  if file_lineno < win.viewport.lines.start or file_lineno > win.viewport.lines.finish then
    return nil
  end

  -- The line is in the viewport and there is no wrapped line
  if win.viewport.lines.finish - win.viewport.lines.start == win.viewport.height then
    return file_lineno - win.viewport.lines.start
  else -- Determine the position in the viewport considering possible prior wrapped lines
    local view_lineno = 0
    for n = win.viewport.lines.start, file_lineno do
      view_lineno = view_lineno + 1
      -- Wrapped line. Shift our line down
      if #win.file.lines[n] > win.viewport.width then
        view_lineno = view_lineno + #win.file.lines[n] // win.viewport.width
      end
    end
    return view_lineno
  end
end

@dther
Copy link
Contributor Author

dther commented Apr 25, 2024

Nope, that is actually preferred. Github doesn't really play nice with it but everyone maintaining vis is used to the patch file/linear history approach.

Fair enough, I'll leave the merge as-is. The git log output looks funny on my end, but I'll live. @fischerling Sorry for changing up the field name just as you posted that...

@fischerling
Copy link
Contributor

@fischerling Sorry for changing up the field name just as you posted that...

No problem I just updated the snippet ;)

dther added 3 commits April 29, 2024 08:36
This allows better control over styling, as well as potential for
entirely new UI elements implemented entirely using the Lua API.
These values are useful for calculating terminal positions.
@rnpnr
Copy link
Collaborator

rnpnr commented Apr 29, 2024

Alright I'm going to merge this, thanks for the patch!

There are a couple unrelated issues that this brings to light though:

First, I was doing some experimenting to add custom lexer keywords (here but it's very hacky) and I noticed that some lexers (eg. markdown) are very close to reaching LEXER_MAX. So that probably needs to be increased.

Second it would be good to actually know from lua what the first available user style id is.

Third, as per your status line example, the function that fills in the status line (in vis-std.lua) should probably be made
configurable so that it doesn't have to be run twice (i.e. store it as a variable in the vis table so that it can get reassigned to your custom function).

Also I'm thinking of a more robust version of win:style_pos that isn't limited to just the window. But that could be added later and win:style_pos would just do the bounds check and call that one.

@rnpnr rnpnr merged commit 1d37e5c into martanne:master Apr 29, 2024
12 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants