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

Add indent string conversion in fmt for multiline string #1264

Merged
merged 4 commits into from
Nov 30, 2024

Conversation

xudyang1
Copy link
Contributor

  • Problem:

The following code outputs spaces instead of tabs '\t':

fmt([[
  line1 (no spaces after dedent)

    line3: -> '  line3' (two spaces)
      line4: -> '    line4' (4 spaces)
]], {}, { dedent = true })

The following does not output tabs:

fmt([[
  line1 (no spaces after dedent)

  \tline3: -> '\\tline3'
  \t\tline4: -> '\\t\\tline3'
]], {}, { dedent = true })
  • Solution:

This PR adds option indent_string to fmt, default to empty string (disabled for backward compatibility).

Users can specify indent_string to let fmt convert them to tabs '\t':

fmt([[
  line1 (no spaces after dedent)

  \tlin3: 1 indent ('\\t' -> '\t')
  \t\tlin4: 2 indent ('\\t\\t' -> '\t\t')
]], {}, { indent_string = [[\t]] })

@xudyang1 xudyang1 force-pushed the feat/fmt-indent branch 3 times, most recently from acc62eb to d8ac2b6 Compare November 27, 2024 23:26
@L3MON4D3
Copy link
Owner

Hi :)
Am I understanding correctly that the issue is essentially that the [[]]-strings can't represent <Tab> as \t?
What about <C-v><Tab>, which inserts a literal tab? Or a manual ([[...]]):gsub("\\t", "\t")? Under which circumstances should a tab not be converted? (Mhmm, maybe if the string itself should insert a "\\t")

Thinking about it a bit more, what do you think about fmt taking an argument which makes it handle control-characters like plain old ""? So we convert [[\t]] to an actual tab, and [[\\t]] to [[\t]]. To me, it seems a bit simpler than sometimes converting tabs and sometimes not :)

@xudyang1
Copy link
Contributor Author

xudyang1 commented Nov 28, 2024

Am I understanding correctly that the issue is essentially that the [[]]-strings can't represent <Tab> as \t?
What about <C-v><Tab>, which inserts a literal tab? Or a manual

Yes, the issue is the difficulty in representing \t in [[]], but it's beyond that.

When I tried to create a custom snippet using fmt, I would like it to auto convert the leading two-space indent to tab when snippet is expanded, so that neovim can determine how to represent indentation in a target buffer (configs related to shiftwidth and expandtab).

For example, let T be a real tab and S be a real space, and with lua config vim.opt.shiftwidth=2 and vim.opt.expandtab=true, the following snippet:

-- cpp.lua for cpp
s({ trig = "test_if" }, {
  sn(1, fmt([[
SSSSif {} then
SSSSSS{}
SSSSend
  ]], { i(1), i(2) },{}))
})
-- when expanded, I would expect
---cpp
---if {} then 
---T{}
---end
-- after that, neovim would expand `T` to certain number of spaces based on configs `shiftwidth`, and `expandtab` (users may have autocmds to set different value of `shiftwidth` for some language)

Currently, LuaSnip does not have spaces-to-tab conversion. I tried to explicitly use \t, but [[\t]] does not work.

  • <C-v><Tab> inserts a true tab character, which works. However, the tedious and complicated key input may introduce unexpected mix of spaces and tabs as indentation, and debugging is difficult as white space characters are not rendered on the screen.
  • ([[...]]):gsub("\\t", "\t" converts all "\\t" to "\t", not only those at the beginning.
  • Under which circumstances should a tab not be converted: this PR only converts indent_string at the beginning of each line, so the following
-- S is a REAL space, T is a REAL tab
-- \t is just [[\t]]

-- with { indent_string="SS" }, example1 expands to:
---echo "hello\tworld\n"
---Techo "new tab \t"

-- with { }, example1 does nothing after dedent:
---echo "hello\tworld\n"
---SSecho "new tab \t"
s({ trig = "example1" }, {
  sn(1, fmt([[
SSSSecho "hello\tworld\n"
SSSSSSecho "new tab \t"
  ]], {},{ indent_string = "SS"}))
})

-- users may want to explicitly showing the indent by \t

-- with { indent_string=[[\t]] }, example2 expands to:
---echo "hello\tworld\n"
---Techo "new tab \t"

-- with { }, example2 does nothing after dedent:
---echo "hello\tworld\n"
---\techo "new tab \t"
s({ trig = "example2" }, {
  -- S is space
  sn(1, fmt([[
TTSSecho "hello\tworld\n"
TTSS\techo "new tab \t"
  ]], {},{ indent_string = [[\t]]}))
})
  • I am not sure letting fmt handle control character may be easier for this case. [[\t]] to "\t" may be expected, but [[string.format("%s\n%s", ...)]] to [[string.format("%s]] .. "\n" .. [[%s", ...)]] when expanded may be undesired:
string.format("%s
%s", ...)

I may not explain that clearly. This PR intends to ensure custom snippets can be formatted in a neat style when using spaces as indent when expandtab=true. (without explicit \t in front of each line or doing multiple text nodes t({ "\t" }))

@xudyang1 xudyang1 force-pushed the feat/fmt-indent branch 2 times, most recently from 034b97f to 3058947 Compare November 28, 2024 03:47
@L3MON4D3
Copy link
Owner

Ah, thank you, that helped me understand the issue :)

I honestly think the best workflow for handling this problem (restore tab-keypress from spaces when expandtab is set) is
inserting real tabs. The debugging-issue you mention can be fixed by using 'listchars', for example

set listchars+=tab:→\ ,trail:␣,space:·
noremap <silent><leader>l :set invlist<cr>

which makes showing whitespace-characters one keybind away.

I'd also prefer real tabs because they can be differentiated from real spaces, in case they should be mixed in the snippet:

int main(int argc,
·········char*[] argv) {
→    printf("Hello World!")
}

where the spaces are dots and the arrow+spaces are a tab
(not possible with indent_string, right?)

Can you see big downsides with this approach? I don't think it's that tedious to press <C-v>, a keybind most programmers should be comfortable with :P

@xudyang1
Copy link
Contributor Author

xudyang1 commented Nov 28, 2024

Ah, thank you, that helped me understand the issue :)

I honestly think the best workflow for handling this problem (restore tab-keypress from spaces when expandtab is set) is inserting real tabs. The debugging-issue you mention can be fixed by using 'listchars', for example

set listchars+=tab:→\ ,trail:␣,space:·
noremap <silent><leader>l :set invlist<cr>

which makes showing whitespace-characters one keybind away.

I'd also prefer real tabs because they can be differentiated from real spaces, in case they should be mixed in the snippet:

int main(int argc,
·········char*[] argv) {
→    printf("Hello World!")
}

where the spaces are dots and the arrow+spaces are a tab (not possible with indent_string, right?)

Can you see big downsides with this approach? I don't think it's that tedious to press <C-v>, a keybind most programmers should be comfortable with :P

I get your point and the method you provided work as expected, but the example is possible with indent_string because indent_string by default does no conversion (indent_string = "")

-- real tab inserted by <C-v> as you suggested
fmt([[
int main(int argc,
·········char*[] argv) {
→    printf("Hello World!")
}
]], {}, {})
-- or explicitly mark tab with indent_string
fmt([[
int main(int argc,
·········char*[] argv) {
\tprintf("Hello World!")
}
]], {}, { indent_string = [[\t]] })

It only offers an opt-in conversion if the user intends to convert something (spaces or [[\t]]) to tab:

fmt([[
int main(int argc,
·········char*[] argv) {
····printf("Hello World!")
}
]], {}, { indent_string = "····"})
-- will become:
[[
int main(int argc,
→    →    ·char*[] argv) {
→    printf("Hello World!")
}
]]

-- but below is not possible with `indent_string = "····"`, as every 4 spaces become a tab
[[
int main(int argc,
→    ·····char*[] argv) {
→    printf("Hello World!")
}
]]
-- to achieve the above goal, the user can explicitly mark tabs with symbols other than space:
fmt([[
int main(int argc,
\t·····char*[] argv) {
\tprintf("Hello World!")
}
]], {}, { indent_string = [[\t]]})
-- or some visual indicator as mark, for example `→`
fmt([[
int main(int argc,
→·····char*[] argv) {
→printf("Hello World!")
}
]], {}, { indent_string = ""}) -- "→" here is not a tab, it is an unicode character

Limitation

indent_string is not able to work with spaces before tabs (a very rare case):

-- <C-v>tab is needed anyway
fmt([[
int main(int argc,
·····→    char*[] argv) {
····printf("Hello World!")
}
]], {}, {})
-- not possible with `indent_string` and explicit marks (though doable if we provide an option to let user specify `unit_indent` and pass it to `convert_indent` function)

@L3MON4D3
Copy link
Owner

Ahh good point :)

Okay, I'm convinced this is a worthwhile addition, thank you!

@L3MON4D3
Copy link
Owner

Okay, I've looked over the code, this looks very solid, I like the tight loops instead of cobbling something together out of :gsub, very minimal :)
Tests also look great (thanks for adding some for dedent), I've no complaints at all

Thank you for implementing this feature!

@L3MON4D3 L3MON4D3 merged commit 6b6d59f into L3MON4D3:master Nov 30, 2024
@xudyang1
Copy link
Contributor Author

Okay, I've looked over the code, this looks very solid, I like the tight loops instead of cobbling something together out of :gsub, very minimal :) Tests also look great (thanks for adding some for dedent), I've no complaints at all

Thank you for implementing this feature!

Thank you for merging this PR. The path I learned how to create custom LuaSnip snippets was from:

s("trigger", {
  t("text node "), i(1, "insert node"), t({ "", "this is next line" }), ...
})

to:

s("trigger", {
  sn(1, fmt([[
  text node {}
  this is next line
  ]], { i(1, "insert node") }, {}))
})

For multiline snippets, a fmt node avoids text nodes and insert nodes interleaving and provides an organized presentation. I really like how it uses a single string as the snippet skeleton and fills the organs with specific nodes, more concise than vscode's multi-string style.

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