[vim/vim] Add 'leadtab' option to 'listchars' (tab guide only in leading indentation) (PR #19094)
Harsh Kapse
Summary
It would be useful to add 'leadtab' option in 'listchars', similar to 'leadmultispace', but for tabs. This allows drawing indentation guides for tabs indented code, without affecting the tabs that appear later in the line.
This is motivated from Neovim feature request: neovim/neovim#37138
Problem
When using hard tabs for indentation (i.e not expandtab), users currently rely on listchar+=tab:.. to visualize tabs. But tab: affects all the tab character, which can look noisy
There already exist leadmultispace: which supports indentation nicely for spaces, but no equivalent for tabs
Proposed Behavior:
Add leadtab:{chars} to 'listchars'
leadtabshould apply only to tabs that occur as leading indentation- once a whitespace is encountered, tabs displays using normal
tab: leadtabshould not affect tabs that occurs after first non whitespace character- Mixed indentation should work naturally, spaces can be rendered with
lead:/leadmultispace:/space:, while leading tabs specifically usesleadtab
Example
With:
set listset listchars=tab:>-,leadtab:+*
Buffer:
\ttexttext\ttab
Expected display (assuming tabstop=4):
- leading tab uses
leadtab::+****text - mid-line tab uses
tab::text>---tab
Test coverage / current work in the pr
I have added tests which should pass when this functionality is added in src/testdir/test_listchars.vim
If the behavior and name is accepted, I would love to work on this
Related:
- Neovim issue: neovim/neovim#37138
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/19094
Commit Summary
- 17b63ff test: added new test for new option 'leadtab'
File Changes
(1 file)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/testdir/test_listchars.vim:
> + normal ggdG + set listchars=tab:>-,leadtab:├─┤ + call append(0, ["\ttext"]) + let expected = ['├──────┤text'] + call Check_listchars(expected, 1, 12) + + " Test leadtab with mixed indentation (spaces + tabs) + normal ggdG + set listchars=tab:>-,leadtab:+*,space:. + call append(0, [" \t text"]) + let expected = ['.+****** text'] + call Check_listchars(expected, 1, 13) + + " Test leadtab without tab set in listchars + normal ggdG + set listchars=leadtab:+*
I don't think leadtab: should be allowed when tab: is not specified. It will complicate many computations.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 commented on this pull request.
In src/testdir/test_listchars.vim:
> + normal ggdG + set listchars=tab:>-,leadtab:├─┤ + call append(0, ["\ttext"]) + let expected = ['├──────┤text'] + call Check_listchars(expected, 1, 12) + + " Test leadtab with mixed indentation (spaces + tabs) + normal ggdG + set listchars=tab:>-,leadtab:+*,space:. + call append(0, [" \t text"]) + let expected = ['.+****** text'] + call Check_listchars(expected, 1, 13) + + " Test leadtab without tab set in listchars + normal ggdG + set listchars=leadtab:+*
ok it do make sense, I will update the tests so it dont let leadtab without tab
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 pushed 2 commits.
- d5724cb feat(diff): added 'leadtab' option in listchars
- 56528d6 test(diff): updated tests for 'leadtab' option
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
zeertzjq
@zeertzjq commented on this pull request.
> @@ -5797,6 +5797,16 @@ A jump table for the options with a short description can be found at |Q_op|. ---+---+--XXX ~ Where "XXX" denotes the first non-blank characters in the line. + lcs-leadtab + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for⬇️ Suggested change
- tabs. When omitted, the "tab" setting is used for + tabs. When omitted, the "tab" setting is used for
In src/errors.h:
> +EXTERN char e_leadtab_requires_tab[]
+ INIT(= N_("E1572: Listchars element leadtab requires tab to be specified"));
This should come after E1571.
> @@ -5797,6 +5797,16 @@ A jump table for the options with a short description can be found at |Q_op|. ---+---+--XXX ~ Where "XXX" denotes the first non-blank characters in the line. + lcs-leadtab + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for + leading tabs. + The "tab" option must be set for this to work.⬇️ Suggested change
- The "tab" option must be set for this to work. + |lcs-tab| must also be set for this to work. *E1571*
In src/testdir/util/gen_opt_test.vim:
> @@ -248,9 +248,9 @@ let test_values = {
\ 'langmap': [['', 'xX', 'aA,bB'], ['xxx']],
\ 'lispoptions': [['', 'expr:0', 'expr:1'], ['xxx', 'expr:x', 'expr:']],
\ 'listchars': [['', 'eol:x', 'tab:xy', 'tab:xyz', 'space:x',
- \ 'multispace:xxxy', 'lead:x', 'leadmultispace:xxxy', 'trail:x',
- \ 'extends:x', 'precedes:x', 'conceal:x', 'nbsp:x', 'eol:\\x24',
- \ 'eol:\\u21b5', 'eol:\\U000021b5', 'eol:x,space:y'],
+ \ 'multispace:xxxy', 'lead:x', 'tab:xy,leadtab:xyz', 'leadmultispace:xxxy',
+ \ 'trail:x', 'extends:x', 'precedes:x', 'conceal:x', 'nbsp:x',
+ \ 'eol:\\x24', 'eol:\\u21b5', 'eol:\\U000021b5', 'eol:x,space:y'],
\ ['xxx', 'eol:']],
Add test that leadtab: alone is not allowed.
- \ ['xxx', 'eol:']], + \ ['xxx', 'eol:', 'leadtab:xyz']],
In src/drawline.c:
> @@ -3258,6 +3259,18 @@ win_line(
{
int tab_len = 0;
long vcol_adjusted = wlv.vcol; // removed showbreak len
+ int lcs_tab1 = wp->w_lcs_chars.tab1;
+ int lcs_tab2 = wp->w_lcs_chars.tab2;
+ int lcs_tab3 = wp->w_lcs_chars.tab3;
+
+ // check if leadtab is set in linechars
⬇️ Suggested change
- // check if leadtab is set in linechars + // check if leadtab is set in 'listchars'
In src/drawline.c:
> @@ -1660,8 +1660,9 @@ win_line( --trailcol; trailcol += (colnr_T)(ptr - line); } - // find end of leading whitespace - if (wp->w_lcs_chars.lead || wp->w_lcs_chars.leadmultispace != NULL) + // find end of leading whitespace or tab
I don't think this comment needs to be changed. Whitespace includes tabs.
In src/errors.h:
> +EXTERN char e_leadtab_requires_tab[]
+ INIT(= N_("E1572: Listchars element leadtab requires tab to be specified"));
Also:
⬇️ Suggested change-EXTERN char e_leadtab_requires_tab[]
- INIT(= N_("E1572: Listchars element leadtab requires tab to be specified"));
+EXTERN char e_leadtab_requires_tab[]
+ INIT(= N_("E1572: 'listchars' field \"leadtab\" requires \"tab\" to be specified"));
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 commented on this pull request.
> @@ -5797,6 +5797,16 @@ A jump table for the options with a short description can be found at |Q_op|. ---+---+--XXX ~ Where "XXX" denotes the first non-blank characters in the line. + lcs-leadtab + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for
Idk what i have to do here? just a space Before 'When'?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
> @@ -5797,6 +5797,16 @@ A jump table for the options with a short description can be found at |Q_op|. ---+---+--XXX ~ Where "XXX" denotes the first non-blank characters in the line. + lcs-leadtab + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for
See :h help-writing:
Vim help files generally use 2 spaces after a sentence (since they are written
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 commented on this pull request.
> @@ -5797,6 +5797,16 @@ A jump table for the options with a short description can be found at |Q_op|. ---+---+--XXX ~ Where "XXX" denotes the first non-blank characters in the line. + lcs-leadtab + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for
oh I get it now. Thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
> + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for + leading tabs.
c... doesn't look right.
- leadtab:c... - leadtab:xy[z] Like |lcs-tab|, but only for leading - tabs. When omitted, the "tab" setting is used for - leading tabs. + leadtab:xy[z] Like |lcs-tab|, but only for leading tabs. When + omitted, the "tab" setting is used for leading tabs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 commented on this pull request.
> + leadtab:c... + leadtab:xy[z] Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for + leading tabs.
ok, not gonna lie i kinda just tried to copy how leadmultispace was documented that's why it had c..., i will update it
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
> + Like |lcs-tab|, but only for leading + tabs. When omitted, the "tab" setting is used for + leading tabs.⬇️ Suggested change
- Like |lcs-tab|, but only for leading - tabs. When omitted, the "tab" setting is used for - leading tabs. + Like |lcs-tab|, but only for leading tabs. When + omitted, the "tab" setting is used for leading tabs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
> @@ -5797,6 +5797,15 @@ A jump table for the options with a short description can be found at |Q_op|. ---+---+--XXX ~ Where "XXX" denotes the first non-blank characters in the line. + *lcs-leadtab* + leadtab:xy[z] + Like |lcs-tab|, but only for leading tabs. When + omitted, the "tab" setting is used for leading tabs. + |lcs-tab| must also be set for this to work. *E1572* + You can combine it with "tab:", for example: + `:set listchars=tab:>-,leadtab:\.`
This example is wrong. I guess you mean this?
⬇️ Suggested change- `:set listchars=tab:>-,leadtab:\.` + `:set listchars=tab:>-,leadtab:.\ `
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/drawline.c:
> #ifdef FEAT_LINEBREAK if (wp->w_p_lbr && wlv.p_extra != NULL && *wlv.p_extra != NUL) wlv.c_extra = NUL; // using p_extra from above else #endif - wlv.c_extra = wp->w_lcs_chars.tab2; - wlv.c_final = wp->w_lcs_chars.tab3; + wlv.c_extra = lcs_tab2;
The comes after the else above the #endif, so its indent should be kept.
- wlv.c_extra = lcs_tab2; + wlv.c_extra = lcs_tab2;
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/screen.c:
> + {
+ return e_leadtab_requires_tab;
+ }
Braces not needed here.
⬇️ Suggested change- {
- return e_leadtab_requires_tab;
- }
+ return e_leadtab_requires_tab;
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/drawline.c:
> + wlv.c_extra = lcs_tab2; #endif⬇️ Suggested change
- wlv.c_extra = lcs_tab2; -#endif +#endif + wlv.c_extra = lcs_tab2;
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 pushed 1 commit.
- 94d41d0 applied review suggetions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq approved this pull request.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Harsh Kapse
@HarshK97 pushed 9 commits.
- a6f0a16 test: added new test for new option 'leadtab'
- 438aa00 feat(diff): added 'leadtab' option in listchars
- f424848 test(diff): updated tests for 'leadtab' option
- 64d255e feat(diff): updated styling in drawline.c
- 77d80f7 feat(diff): added guard to not let users use 'leadtab' without 'tab' also set
- fb93b37 test(diff): added entries of 'leadtab' in gen_opt_test.vim and test_options.vim
- 7a37c66 runtime(doc): updated the docs for the new option 'leadtab'
- af80c7c test(leadtab): updated the tests
- 2369214 applied review suggetions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.