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

Fix ueberzug backend's y position being off by one #6

Closed
wants to merge 2 commits into from

Conversation

Vonr
Copy link

@Vonr Vonr commented Jul 11, 2023

Before:
image
After:
image

@3rd
Copy link
Owner

3rd commented Jul 11, 2023

@Vonr Hey, thank you so much for the PR!
I don't think it's a ueberzug-specific issue, it's probably something related to accounting for user-defined options around here:

local get_global_offsets = function()

Been seeing the same issue on #5

@Vonr
Copy link
Author

Vonr commented Jul 11, 2023

That's interesting... So far all the images I've tested with work with this fix but ill look into renderer.lua.

@Vonr
Copy link
Author

Vonr commented Jul 11, 2023

Might be line 109?

@Vonr
Copy link
Author

Vonr commented Jul 11, 2023

local get_global_offsets = function()
  local x = 0
  local y = 0
  if vim.opt.number then x = x + math.max(vim.opt.numberwidth:get(), ('' .. vim.api.nvim_buf_line_count(0)):len() + (vim.opt.relativenumber:get() and 1 or 0)) end
  -- if vim.opt.signcolumn:get() ~= "no" then x = x + 2 end
  if vim.opt.showtabline == 2 then y = y + 1 end
  if vim.opt.winbar ~= "none" then y = y + 1 end
  return { x = x, y = y }
end

This fixes my left-right alignment but it probably breaks signcolumn alignment.
I commented that line out because I use signcolumn 'number'.

@Vonr
Copy link
Author

Vonr commented Jul 11, 2023

local get_global_offsets = function()
  local x = 0
  local y = 0

  local numberwidth = math.max(
      vim.opt.numberwidth:get(),
      ('' .. vim.api.nvim_buf_line_count(0)):len()
        + (vim.opt.relativenumber:get() and 1 or 0))

  if vim.opt.number then x = x + numberwidth end

  local signcolumn = vim.opt.signcolumn:get()
  if signcolumn ~= "no" then
      x = x + math.max(0, 2 - (signcolumn == "number" and numberwidth or 0))
  end

  if vim.opt.showtabline == 2 then y = y + 1 end
  if vim.opt.winbar ~= "none" then y = y + 1 end
  return { x = x, y = y }
end

This has a pretty decent chance of working for most setups, but I honestly have no idea.

@3rd
Copy link
Owner

3rd commented Jul 11, 2023

@Vonr that looks great, I'll test it out today and also add some unit tests for this, and we can merge it soon.
Thanks a lot!


local numberwidth = math.max(
vim.opt.numberwidth:get(),
('' .. vim.api.nvim_buf_line_count(0)):len()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are buffer-relative and this version applies the offsets for the active buffers to all the visible windows, looking into making this generic enough.

@@ -106,7 +117,7 @@ local render = function(image)
-- global offsets
local global_offsets = get_global_offsets()
x_offset = global_offsets.x - window.scroll_x
y_offset = global_offsets.y + 1 - window.scroll_y
y_offset = global_offsets.y - window.scroll_y
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, didn't see it because the winbar logic above is wrong.

@3rd
Copy link
Owner

3rd commented Jul 11, 2023

@Vonr think we can rely on textoff for the gutter math, can you test if it works ok for you with the latest changes?
c957678

@Vonr
Copy link
Author

Vonr commented Jul 12, 2023

Looks like it works, thank you!

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.

2 participants