pragtical / pragtical

The practical and pragmatic code editor.
https://pragtical.dev
MIT License
470 stars 19 forks source link

Hard Tab / Indent not aligned? #61

Open starfishpatkhoo opened 9 months ago

starfishpatkhoo commented 9 months ago

Hi all,

I can't quite figure out where this issue goes, so apologies if this should be considered upstream, or has been addressed elsewhere.

This applies to "tabs: 4" as indent size for current file.

Basically, if I have some text, press tab, and enter more text, I expect that using a bunch of \t should allow me to align text up accordingly. But that's not the case.

image

This behaviour of course does not happen with Windows notepad or GNU nano - both of which are nicely aligned. This happens on a new blank document, and it happens when opening an existing document that was properly indented/aligned from elsewhere..

Is this related to https://github.com/lite-xl/lite-xl/issues/789 ?

Anyone have a next step for me to follow on this issue?

jgmdev commented 9 months ago

If I understand correctly... It seems that nano calculates the amount of characters before a \t sequence and sets the tab size dynamically based on that (eg: (tab_size - (tab_position - 1) % tab_size) * space_width). Meanwhile pragtical/lite-xl will just set a fixed tab size based on the width of the space character (eg: tab_size * space_width).

As an example:

1234123\t\t hello

would expand in nano to:

1234123      hello

First tab will be set to the width of 1 space to complement the missing 8th character while the second tab would be set to 4 full spaces to keep the group sequence of 4 characters.

Meanwhile on pragtical/lite-xl it would just statically set the width of the tab to 4 spaces:

1234123         hello

This could be implemented, but as far as I can think right now about how the renderer handles tab sizing, it would maybe need some substantial amount of changes. And since the editor supports fallback fonts that can have varied character width sizes it could represent a challenge doing it right (meaning working well with different combinations of font)...

Without the complications above, we could replace the tab characters by the required amount of spaces when rendering the tokenizer output using the given example formula (tab_size - (tab_position_on_line - 1) % tab_size) * space_width but we would also need to keep track of the already processed tabs length and take into consideration utf8 character sequences longer than 1 byte.

Is this related to lite-xl/lite-xl#789 ?

Not entirely related to this issue but has some relationship to it.

starfishpatkhoo commented 8 months ago

Yeah, that about sums it up. I don't know, I'm a bit OCD, so I like to align comments up for certain things. Like a whole bunch of defaults, or variables/properties.. If all the comments are aligned, its easier to read...

Tried with 3.2.2 but issue is still there..

Is it easier to somehow maintain a bunch of (pre-calculated) indent positions instead of tab sizes? Like if tab size is 4, so it would be 0,4,8,12,16,... So pressing tab would move to the next indent position, filling it with a \t or spaces as the case may be (hard or soft tabs). This indent position would be (re-)calculated whenever a document is (re-)loaded, or if the font used changes (fallback),

Is this easier to do?

juliardi commented 5 months ago

Hi all, the lack of this feature also bugging me. Then after reading you discussion and tried to take a look at Lite XL and Pragtical's source code, I think I can implement this feature as a plugin by overriding doc:get_indent_string() and doc:get_indent_text(). I am a beginner at Lua, so I don't know if this is the correct approach. But I know that this approach is not perfect, because it doesn't take care of the problem of varied characters width size described by @jgmdev above. Also, it only works when the indent type is a bunch of space, not a real tab.

Basically the plugin just re-implement the doc:get_indent_string() by adding 2 parameters and a calculation to determine how many white space should be added as an indent string based on the position of the caret(?, sorry I don't know the right term). As for the doc:get_indent_text(), it just rearrange the first 3 variable initialization.

Here is the plugin code :

--mod-version:3

local doc = require "core.doc"

function doc:get_indent_string(start_column, in_beginning_whitespace)
  local indent_type, indent_size = self:get_indent_info()
  if indent_type == "hard" then
    return "\t"
  end

  if in_beginning_whitespace then
    return string.rep(" ", indent_size)
  end

  local col_modulo = start_column % indent_size

  return string.rep(" ", indent_size - col_modulo)
end

function doc:indent_text(unindent, line1, col1, line2, col2)
  local _, se = self.lines[line1]:find("^[ \t]+")
  local in_beginning_whitespace = col1 == 1 or (se and col1 <= se + 1)
  local text = self:get_indent_string(col1, in_beginning_whitespace)
  local has_selection = line1 ~= line2 or col1 ~= col2
  if unindent or has_selection or in_beginning_whitespace then
    local l1d, l2d = #self.lines[line1], #self.lines[line2]
    for line = line1, line2 do
      if not has_selection or #self.lines[line] > 1 then -- don't indent empty lines in a selection
        local e, rnded = self:get_line_indent(self.lines[line], unindent)
        self:remove(line, 1, line, (e or 0) + 1)
        self:insert(line, 1,
          unindent and rnded:sub(1, #rnded - #text) or rnded .. text)
      end
    end
    l1d, l2d = #self.lines[line1] - l1d, #self.lines[line2] - l2d
    if (unindent or in_beginning_whitespace) and not has_selection then
      local start_cursor = (se and se + 1 or 1) + l1d or #(self.lines[line1])
      return line1, start_cursor, line2, start_cursor
    end
    return line1, col1 + l1d, line2, col2 + l2d
  end
  self:insert(line1, col1, text)
  return line1, col1 + #text, line1, col1 + #text
end

And this is the result :

Redlolz commented 5 months ago

Made an issue on the Lite-XL repo (https://github.com/lite-xl/lite-xl/issues/1780) and was informed that there was already an issue open for this (https://github.com/lite-xl/lite-xl/issues/953), but that one has been open for around two years already and I don't think it's getting implemented there anytime soon. Would love to see this implemented in Pragtical.

I saw in renderer.c that there was a function ren_font_group_set_tab_size that sets tab_advance to be n times space_advance, is this what I'd need to change or am I missing something?