[vim/vim] runtime(tohtml): convert tohtml plugin to Vim9 script (PR #19915)
Mao-Yining
I spent some time converting tohtml to vim9, initially to see if there would be any performance improvements.
But I'm not sure if it's an environment issue or something else — the converted version seems to perform worse than the original, and the progress bar during conversion doesn't show up either. So please do not merge for now.
I'd like to hear some feedback from everyone, and also have the tests run.
Thanks all!
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/19915
Commit Summary
- cfeb66d runtime(tohtml): convert tohtml plugin to Vim9 script
File Changes
(3 files)
- M runtime/autoload/tohtml.vim (1484)
- M runtime/plugin/tohtml.vim (491)
- M runtime/syntax/2html.vim (3225)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)Emm... Does the maintainer has a gihub account?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 86f53b9 runtime(tohtml): convert tohtml plugin to Vim9 script
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Enno
Konfekt left a comment (vim/vim#19915)You could mark it as draft for now?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 8ee7d85 runtime(tohtml): convert tohtml plugin to Vim9 script
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 423896c runtime(tohtml): convert tohtml plugin to Vim9 script
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19915)It used to be @fritzophrenic
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#19915)@mao-yining
I profiled the converted script and found that the main line-processing loop (the while lnum <= end loop) runs as top-level script code, which is interpreted even in Vim9 script. Only code inside def functions is compiled to bytecode.
Wrapping the main loop in a def ProcessLines() function reduced execution time from ~37s to ~15s (2.5x speedup) on a 2000+ line file.
Changes:
- Wrapped the main loop in
def ProcessLines()and call it immediately after definition - Moved
all_linesandstart_lnumvariables inside the function - Fixed Vim9 syntax issues required inside
def:- Added space around
:in ternary operator - Added space around
-inlnum - 1 - Changed
tablistfromlist<string>tolist<number>usingmapnew()+str2nr(), since implicit string-to-number conversion is not allowed insidedef
- Added space around
diff --git a/runtime/syntax/2html.vim b/runtime/syntax/2html.vim index 026406dc6..306ca9af0 100644 --- a/runtime/syntax/2html.vim +++ b/runtime/syntax/2html.vim @@ -1512,7 +1512,11 @@ if !settings.expand_tabs setlocal isprint+=9 endif -while lnum <= end +def ProcessLines() + var all_lines = getline(lnum, end) + var start_lnum = lnum + + while lnum <= end # If there are filler lines for diff mode, show these above the line. Add_diff_fill(lnum) @@ -1535,7 +1539,7 @@ while lnum <= end endif # put numcol in a separate group for sake of unselectable text - new = (settings.number_lines ? HtmlFormat_n(numcol, FOLDED_ID, 0, lnum): "") .. HtmlFormat_t(new, FOLDED_ID, 0) + new = (settings.number_lines ? HtmlFormat_n(numcol, FOLDED_ID, 0, lnum) : "") .. HtmlFormat_t(new, FOLDED_ID, 0) # Skip to the end of the fold var new_lnum = foldclosedend(lnum) @@ -1550,12 +1554,12 @@ while lnum <= end # # A line that is not folded, or doing dynamic folding. # - var line = getline(lnum) + var line = all_lines[lnum - start_lnum] var len = strlen(line) if settings.dynamic_folds # First insert a closing for any open folds that end on this line - while !empty(foldstack) && get(foldstack, 0).lastline == lnum-1 + while !empty(foldstack) && get(foldstack, 0).lastline == lnum - 1 new ..= "</span></span>" remove(foldstack, 0) endwhile @@ -1710,9 +1714,9 @@ while lnum <= end if settings.expand_tabs var offset = 0 var idx = stridx(expandedtab, "\t") - var tablist = split(&vts, ',') + var tablist = mapnew(split(&vts, ','), (_, v) => str2nr(v)) if empty(tablist) - tablist = [ &ts ] + tablist = [&ts] endif var tabidx = 0 var tabwidth = 0 @@ -1789,6 +1793,8 @@ while lnum <= end pgb.Incr() endif endwhile +enddef +ProcessLines() # Diff filler is returned based on what needs inserting *before* the given line. # So to get diff filler at the end of the buffer, we need to use last line + 1
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 2 commits.
- 71dd73d perf: use def to compile heat code to bytecode
- 3cf7ced perf: use def to compile into bytecode
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)@h-east Thanks for your suggestion. I've also optimized another similar place.
Does anyone have a better solution for this kind of code?
let s:build_fun_lines[-1] =<< trim ENDLET blablabla ENDLET
My current solution is:
trim_tmp =<< trim ENDLET blablabla ENDLET build_fun_lines += trim_tmp
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- b71e3ae fix: can't show b_color
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 655f3e4 fix: correct synIDattr boolean checks and progress bar logic
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 958724f fix: correct synIDattr boolean checks and progress bar logic
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- ea8391c fix(tohtml): fix variable scope and initialization in line loop
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- d1fbd1b change document
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)Thanks guys, this sounds very promising.
I haven't been active for a few years on this. I do have a limited test suite over here along with where I had been officially maintaining the script but I don't think I had any work in progress that isn't already included in the officially distributed runtime: https://sourceforge.net/p/vim-tohtml/code/ci/default/tree/
The tests aren't complete yet, I have a couple tests for very basic functionality (and checking the output is valid HTML) but mostly I had been adding tests for new features or bugfixes as I implemented them. Still, it's a lot better than nothing and we should make sure there are not any new test failures comparing the current version to the Vim9 script version.
Vim9 script is something I wanted to eventually include (see https://sourceforge.net/p/vim-tohtml/issues/28/ from years ago) but I never got around to experimenting with it even outside the plugin so once this MR is ready to go and is properly tested for regressions I think it should definitely merge.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)@fritzophrenic Seems this require java environment which I am not familiarize with. Could you please test it?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)Yeah I'll try to give that a try tonight or later in the week. I need to get set up again. It's using java only for running a standalone HTML validator as a test step, not to write or run the tests themselves.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)Hey I got the test running (and fixed a test issue caused by a change to diff highlighting sometime in the last few years). After downloading the changes for this pull request, I get several exceptions thrown during the test run, leading to test failures. I'll see if I can extract the exact option combinations leading to each error later, but for now here are the errors:
Caught exception in Test_line_numbers_with_filler(): Vim(eval):E1001: Variable not found: saved_style @ command line..script /home/ben/code/vim-src/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_line_numbers_with_filler[7]..tohtml#Convert2HTML[11]..script /home/ben/code/vim-tohtml/syntax/2html.vim[1799]..function <SNR>20_ProcessLines[7]..<SNR>20_Add_diff_fill[56]..<SNR>20_HtmlFormat_d[1]..<SNR>20_HtmlFormat[29]..<SNR>20_BuildStyleWrapper, line 30
The same error occurs in Test_whole_filler_disabled_one_file(), Test_whole_filler_disabled_two_files(), Test_whole_filler_disabled_uncopyable(), Test_whole_filler_enabled_one_file(), and Test_whole_filler_enabled_two_files(). So there's a good chance this issue is related to diff mode in some way.
Caught exception in Test_default_expandtab_from_foldcol(): Vim(eval):E1050: Colon required before a range: %foldclose! @ command line..script /home/ben/code/vim-src/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_default_expandtab_from_foldcol[6]..tohtml#Convert2HTML[11]..script /home/ben/code/vim-tohtml/syntax/2html.vim[1460]..function <SNR>20_ProcessFolds, line 36
This error occurs in many tests across several test files so I think it is related to building the dynamic fold column in general. Affected tests:
- Test_default_expandtab_from_foldcol()
- Test_default_expandtab_from_linenums()
- Test_default_expandtab_from_setting()
- Test_default_expandtab_from_tabstop()
- Test_default_expandtab_from_vartabs()
- Test_default_keep_tabs()
- Test_expand_normal_tab()
- Test_keep_vartab()
- Test_direct_hi_rules()
- Test_linked_hi_rules()
- Test_all_variables_use_var()
- Test_all_regions()
- Test_diff_filler()
- Test_fold_column_fallback()
- Test_fold_column_generated_only()
- Test_fold_column_input_only()
- Test_fold_text()
- Test_folding_disabled_fallback()
- Test_folding_disabled_generated()
- Test_folding_disabled_inputs()
- Test_invalid()
- Test_line_numbers()
- Test_lnum_no_foldcolumn_fallback()
- Test_lnum_no_foldcolumn_generated()
- Test_lnum_no_foldcolumn_inputs()
- Test_no_lnums_in_all()
- Test_no_lnums_in_fallback()
- Test_no_lnums_in_none()
- Test_script_tag_highlander()
- Test_script_tag_linenums()
- Test_script_tag_omitted()
Caught exception in Test_script_version_nominal(): Vim(eval):E1001: Variable not found: lines @ command line..script /home/ben/code/vim-src/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_script_version_nominal[1]..script /home/ben/code/vim-tohtml/syntax/2html.vim[1799]..function <SNR>18_ProcessLines[7]..<SNR>18_Add_diff_fill, line 60
Also in test Test_script_version_piecemeal().
Caught exception in Test_nodoc_html_header_footer(): Vim(eval):E121: Undefined variable: win_list @ command line..script /home/ben/code/vim-src/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_nodoc_html_header_footer[13]..tohtml#Convert2HTML, line 15
Also impacts: Test_nodoc_links(), Test_nodoc_modeline_nodoc(), Test_script_tag_blank_linenums(), and Test_script_tag_dyn_folds().
I'll try to provide more info this weekend if I have time. If you want to run on your own, you just need to install a Java JRE and set up to build Vim from source so you're using the latest and greatest. I have the Vim repository checked out to ~/code/vim-src and my TOhtml repository checked out to ~/code/vim-tohtml. I downloaded the vnu.jar file to ~/code/vim-tohtml/testdir and installed OpenJDK JRE 25 from the Linux Mint software manager, then all it took was a "make" command to run the tests.
Hope that help, this looks like a really good start!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
@fritzophrenic commented on this pull request.
> -" 7.3_v5 ( unreleased ): Fix bug with 'nowrapscan' and also with out-of-sync
-" folds in diff mode when first line was folded.
-" 7.3_v4 (Vim 7.3.0000): Bugfixes, especially for xhtml markup, and diff mode
-" 7.3_v3 (Vim 7.3.0000): Refactor option handling and make html_use_css
-" default to true when not set to anything. Use strict
-" doctypes where possible. Rename use_xhtml option to
-" html_use_xhtml for consistency. Use .xhtml extension
-" when using this option. Add meta tag for settings.
-" 7.3_v2 (Vim 7.3.0000): Fix syntax highlighting in diff mode to use both the
-" diff colors and the normal syntax colors
-" 7.3_v1 (Vim 7.3.0000): Add conceal support and meta tags in output
-"}}}
-"}}}
+#
+# Changelog: {{{
+# 9.0_v2 (this version): - Warn if using deprecated g:use_xhtml option
I updated the changelog here, in case you want to pull that in while we figure out the test failures: https://sourceforge.net/p/vim-tohtml/code/ci/default/tree/plugin/tohtml.vim
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- e37acf6 Update Changelog
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining commented on this pull request.
> -" 7.3_v5 ( unreleased ): Fix bug with 'nowrapscan' and also with out-of-sync
-" folds in diff mode when first line was folded.
-" 7.3_v4 (Vim 7.3.0000): Bugfixes, especially for xhtml markup, and diff mode
-" 7.3_v3 (Vim 7.3.0000): Refactor option handling and make html_use_css
-" default to true when not set to anything. Use strict
-" doctypes where possible. Rename use_xhtml option to
-" html_use_xhtml for consistency. Use .xhtml extension
-" when using this option. Add meta tag for settings.
-" 7.3_v2 (Vim 7.3.0000): Fix syntax highlighting in diff mode to use both the
-" diff colors and the normal syntax colors
-" 7.3_v1 (Vim 7.3.0000): Add conceal support and meta tags in output
-"}}}
-"}}}
+#
+# Changelog: {{{
+# 9.0_v2 (this version): - Warn if using deprecated g:use_xhtml option
Thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 310b3e2 fix some problem, but diff is still mal-functions
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)@h-east
This will output <c2>: echo 'vert:│,fold:·,trunc:…'['vert:│,fold:·,trunc:…'->matchend('fold:')]
But vim9cmd echo 'vert:│,fold:·,trunc:…'['vert:│,fold:·,trunc:…'->matchend('fold:')] will output t.
Is this a bug or intentionally design?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
@fritzophrenic commented on this pull request.
> - let s:newfold = {'firstline': s:lnum, 'lastline': foldclosedend(s:lnum), 'level': s:level,'type': "closed-fold"}
- " only add the fold if we don't already have it
- if empty(s:allfolds) || index(s:allfolds, s:newfold) == -1
- let s:newfold.type = "open-fold"
- call add(s:allfolds, s:newfold)
- endif
- " open the fold so we can find any contained folds
- execute s:lnum.."foldopen"
- else
- if !s:settings.no_progress
- call s:pgb.incr()
- if s:pgb.needs_redraw
- redrawstatus
- let s:pgb.needs_redraw = 0
+ # close all folds to get info for originally open folds
+ silent! %foldclose!
⬇️ Suggested change
- silent! %foldclose! + silent! :%foldclose!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> - let afold.firstline = g:html_start_line - else - " if the fold lies outside the range or the start and stop enclose - " the entire range, don't bother parsing it - call remove(s:allfolds, index(s:allfolds, afold)) - let removed = 1 - if afold.lastline > g:html_end_line - let leveladjust += 1 + # sort the folds so that we only ever need to look at the first item in the + # list of folds + sort(allfolds, FoldCompare) + + &l:foldtext = foldtext_save + + # close all folds again so we can get the fold text as we go + silent! %foldclose!
- silent! %foldclose! + silent! :%foldclose!
Fixes one of the test failure exceptions.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> + # <input>s have a background color (maybe not really a bug, this isn't + # well-defined) + # + # use strwidth, because we care only about how many character boxes are + # needed to size the input, we don't care how many characters (including + # separately counted composing chars, from strchars()) or bytes (from + # len())the string contains. strdisplaywidth() is not needed because none of + # the unselectable groups can contain tab characters (fold column, fold + # text, line number). + # + # Note, if maxlength property needs to be added in the future, it will need + # to use strchars(), because HTML specifies that the maxlength parameter + # uses the number of unique codepoints for its limit. + trim_tmp =<< trim eval ENDLET + if make_unselectable + var return_span = "<span " .. extra_attrs .. 'class="' .. style_name .. diffstyle .. '"'
The goal of the diffstyle variable is to optimize the function we are defining dynamically so that we don't need to evaluate any comparison logic for the diff style ID if we are not in diff mode at all. So inside the "trim eval" block in the original code, diffstyle was in curly brackets so that it would be evaluated when defining the string rather than when running the function. This was accidentally omitted during the vim9script conversion, leading to a variable not defined exception.
Additionally, the diffstyle variable will either be a string ending in ".." already, or an empty string. Adding another ".." afterwards is a syntax error.
Fix both these exceptions.
⬇️ Suggested change- var return_span = "<span " .. extra_attrs .. 'class="' .. style_name .. diffstyle .. '"'
+ var return_span = "<span " .. extra_attrs .. 'class="' .. style_name .. {diffstyle} '"'
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> ENDLET
- if s:settings.use_input_for_pc !=# 'none'
- call add(s:wrapperfunc_lines, [])
- let s:wrapperfunc_lines[-1] =<< trim eval ENDLET
- let return_span ..= '<input'..s:unselInputType..' class="' .. l:style_name ..{s:diffstyle}'"'
- let return_span ..= ' value="'..substitute(a:unformatted,'\s\+$',"","")..'"'
- let return_span ..= " onselect='this.blur(); return false;'"
- let return_span ..= " onmousedown='this.blur(); return false;'"
- let return_span ..= " onclick='this.blur(); return false;'"
- let return_span ..= " readonly='readonly'"
- let return_span ..= ' size="'..strwidth(a:unformatted)..'"'
- let return_span ..= (s:settings.use_xhtml ? '/>' : '>')
+ wrapperfunc_lines += trim_tmp
+ if settings.use_input_for_pc !=# 'none'
+ trim_tmp =<< trim eval ENDLET
+ return_span ..= '<input' .. unselInputType .. ' class="' .. style_name .. diffstyle .. '"'
The goal of the diffstyle variable is to optimize the function we are defining dynamically so that we don't need to evaluate any comparison logic for the diff style ID if we are not in diff mode at all. So inside the "trim eval" block in the original code, diffstyle was in curly brackets so that it would be evaluated when defining the string rather than when running the function. This was accidentally omitted during the vim9script conversion, leading to a variable not defined exception.
Additionally, the diffstyle variable will either be a string ending in ".." already, or an empty string. Adding another ".." afterwards is a syntax error.
Fix both these exceptions.
⬇️ Suggested change- return_span ..= '<input' .. unselInputType .. ' class="' .. style_name .. diffstyle .. '"'
+ return_span ..= '<input' .. unselInputType .. ' class="' .. style_name .. {diffstyle} '"'
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> endif
- call add(s:wrapperfunc_lines, [])
- let s:wrapperfunc_lines[-1] =<< trim eval ENDLET
- return return_span..'</span>'
- else
- return "<span "..a:extra_attrs..'class="' .. l:style_name .. {s:diffstyle}'">'..a:text.."</span>"
- endif
+ trim_tmp =<< trim eval ENDLET
+ return return_span .. '</span>'
+ else
+ return "<span " .. extra_attrs .. 'class="' .. style_name .. diffstyle .. '">' .. text .. "</span>"
The goal of the diffstyle variable is to optimize the function we are defining dynamically so that we don't need to evaluate any comparison logic for the diff style ID if we are not in diff mode at all. So inside the "trim eval" block in the original code, diffstyle was in curly brackets so that it would be evaluated when defining the string rather than when running the function. This was accidentally omitted during the vim9script conversion, leading to a variable not defined exception.
Additionally, the diffstyle variable will either be a string ending in ".." already, or an empty string. Adding another ".." afterwards is a syntax error.
Fix both these exceptions.
⬇️ Suggested change- return "<span " .. extra_attrs .. 'class="' .. style_name .. diffstyle .. '">' .. text .. "</span>"
+ return "<span " .. extra_attrs .. 'class="' .. style_name .. {diffstyle} '">' .. text .. "</span>"
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> + new = new .. repeat(difffillchar, 3)
+ endif
+ new = HtmlFormat_d(new, DIFF_D_ID, 0)
+ if settings.number_lines
+ # Indent if line numbering is on. Indent gets style of line number
+ # column.
+ new = HtmlFormat_n(repeat(' ', margin), LINENR_ID, 0, 0) .. new
+ endif
+ if settings.dynamic_folds && !settings.no_foldcolumn
+ if foldcolumn > 0
+ # Indent for foldcolumn if there is one. Assume it's empty, there should
+ # not be a fold for deleted lines in diff mode.
+ new = FoldColumn_fill() .. new
+ endif
+ endif
+ build_fun_lines += trim_tmp
This line causes an exception because build_fun_lines is not defined nor used to build this function. The easy fix is to delete the build_fun_lines += trim_tmp line as you seem to have removed the dynamic building of this function in favor of defining the entire function to always have the same logic.
This may be fine, but this does remove the optimization that was here previously. Was that intentional? This function was previously built dynamically similar to other functions which run frequently in the inner loop. This way we can avoid evaluating logic during the loop processing when it only depends on the tohtml settings (which we know up-front before processing the file in any way). The better fix might be to go back to building the function using the build_fun_lines method? I don't know how much time savings this particular function actually gives us, though.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> + endif
+ new = HtmlFormat_d(new, DIFF_D_ID, 0)
+ if settings.number_lines
+ # Indent if line numbering is on. Indent gets style of line number
+ # column.
+ new = HtmlFormat_n(repeat(' ', margin), LINENR_ID, 0, 0) .. new
+ endif
+ if settings.dynamic_folds && !settings.no_foldcolumn
+ if foldcolumn > 0
+ # Indent for foldcolumn if there is one. Assume it's empty, there should
+ # not be a fold for deleted lines in diff mode.
+ new = FoldColumn_fill() .. new
+ endif
+ endif
+ build_fun_lines += trim_tmp
+ add(lines, new .. HtmlEndline)
After fixing the other exceptions, the add(lines, new .. HtmlEndLine) line throws an exception because "lines" is not defined. I don't understand this one, because "lines" is defined up at line 827, with var lines = [].
This is the first time I've worked with vim9script, but I think I understand from the help files that this should define lines as a script-local variable. And since our function here is written in vim9script, it should be able to access the "lines" variable just fine. I'm looking at the text just above :help E1269:
If the script the function is defined in is Vim9 script, then script-local
variables can be accessed without the "s:" prefix. They must be defined
before the function is compiled. If the script the function is defined in is
legacy script, then script-local variables must be accessed with the "s:"
prefix if they do not exist at the time of compiling.
*E1269*
Script-local variables in a |Vim9| script must be declared at the script
level. They cannot be created in a function, also not in a legacy function.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)> previously. Was that intentional? This function was previously built
> dynamically similar to other functions which run frequently in the inner
> loop. This way we can avoid evaluating logic during the loop processing
> when it only depends on the tohtml settings (which we know up-front
> before processing the file in any way). The better fix might be to go
> back to building the function using the build_fun_lines method?
"lines", I previously thought it is because of execute(), but that seems
not the reason.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
> + # <input>s have a background color (maybe not really a bug, this isn't + # well-defined) + # + # use strwidth, because we care only about how many character boxes are + # needed to size the input, we don't care how many characters (including + # separately counted composing chars, from strchars()) or bytes (from + # len())the string contains. strdisplaywidth() is not needed because none of + # the unselectable groups can contain tab characters (fold column, fold + # text, line number). + # + # Note, if maxlength property needs to be added in the future, it will need + # to use strchars(), because HTML specifies that the maxlength parameter + # uses the number of unique codepoints for its limit. + trim_tmp =<< trim eval ENDLET + if make_unselectable + var return_span = "<span " .. extra_attrs .. 'class="' .. style_name .. diffstyle .. '"'
In such cases, using interpolated-string might be a good idea.
:h interpolated-string
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#19915)@h-east This will output
<c2>:echo 'vert:│,fold:·,trunc:…'['vert:│,fold:·,trunc:…'->matchend('fold:')]Butvim9cmd echo 'vert:│,fold:·,trunc:…'['vert:│,fold:·,trunc:…'->matchend('fold:')]will outputt. Is this a bug or intentionally design?
What's more. All of these isn't what I expect. This is for extracting
·infold:·.
This is a specification of Vim9 script.
See ":help vim9-string-index"
vim9cmd echo 'vert:│,fold:·,trunc:…'[charidx('vert:│,fold:·,trunc:…', matchend('vert:│,fold:·,trunc:…', 'fold:'))]
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
@fritzophrenic commented on this pull request.
> + endif
+ new = HtmlFormat_d(new, DIFF_D_ID, 0)
+ if settings.number_lines
+ # Indent if line numbering is on. Indent gets style of line number
+ # column.
+ new = HtmlFormat_n(repeat(' ', margin), LINENR_ID, 0, 0) .. new
+ endif
+ if settings.dynamic_folds && !settings.no_foldcolumn
+ if foldcolumn > 0
+ # Indent for foldcolumn if there is one. Assume it's empty, there should
+ # not be a fold for deleted lines in diff mode.
+ new = FoldColumn_fill() .. new
+ endif
+ endif
+ build_fun_lines += trim_tmp
+ add(lines, new .. HtmlEndline)
@h-east do you know why accessing the script-local "lines" variable does not work here?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 3 commits.
- 9ad1063 revert changes for Add_diff_fill
- b328df7 fix: foldfillchar
- 6e4d92e use '..=' istead of 'a = a ..'
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 2 commits.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining commented on this pull request.
> + endif
+ new = HtmlFormat_d(new, DIFF_D_ID, 0)
+ if settings.number_lines
+ # Indent if line numbering is on. Indent gets style of line number
+ # column.
+ new = HtmlFormat_n(repeat(' ', margin), LINENR_ID, 0, 0) .. new
+ endif
+ if settings.dynamic_folds && !settings.no_foldcolumn
+ if foldcolumn > 0
+ # Indent for foldcolumn if there is one. Assume it's empty, there should
+ # not be a fold for deleted lines in diff mode.
+ new = FoldColumn_fill() .. new
+ endif
+ endif
+ build_fun_lines += trim_tmp
+ add(lines, new .. HtmlEndline)
Seems has been solved
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)Attaching log from the test run after the recent changes (cleaned up some for readability). The "lines" issue is still there, but we have resolved enough issues to get some interesting feedback from the HTML validator and other checks now as well. I probably won't have much time to dig into this over the weekend but can keep plinking away during the week.
test.log
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)I don't have access to the internet during the week. I'll take another look next weekend. I also have no idea about the lines issue.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
@fritzophrenic commented on this pull request.
> + var lines = readfile(main_plugin_path, "", 20)
+ filter(lines, (_, val) => val =~ "loaded_2html_plugin = ")
+ if empty(lines)
+ g:unloaded_tohtml_plugin = "unknown"
else
- let g:unloaded_tohtml_plugin = substitute(s:lines[0], '.*loaded_2html_plugin = \([''"]\)\(\%(\1\@!.\)\+\)\1', '\2', '')
+ g:unloaded_tohtml_plugin = substitute(lines[0], '.*loaded_2html_plugin = \([''"]\)\(\%(\1\@!.\)\+\)\1', '\2', '')
I don't know why this fixes the exception for the "lines" variable in Add_diff_fill(), but it does. It looks like somehow having a variable of the same name here inside an if/else block prevents the script-level variable being seen inside the function. I don't know if this is expected behavior of Vim or a bug, but anyway this fixes the exception.
⬇️ Suggested change- var lines = readfile(main_plugin_path, "", 20) - filter(lines, (_, val) => val =~ "loaded_2html_plugin = ") - if empty(lines) - g:unloaded_tohtml_plugin = "unknown" - else - let g:unloaded_tohtml_plugin = substitute(s:lines[0], '.*loaded_2html_plugin = \([''"]\)\(\%(\1\@!.\)\+\)\1', '\2', '') - g:unloaded_tohtml_plugin = substitute(lines[0], '.*loaded_2html_plugin = \([''"]\)\(\%(\1\@!.\)\+\)\1', '\2', '') + var plugin_lines = readfile(main_plugin_path, "", 20) + filter(plugin_lines, (_, val) => val =~ "loaded_2html_plugin = ") + if empty(lines) + g:unloaded_tohtml_plugin = "unknown" +else + g:unloaded_tohtml_plugin = substitute(plugin_lines[0], '.*loaded_2html_plugin = \([''"]\)\(\%(\1\@!.\)\+\)\1', '\2', '')
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)New log of entire test suite after fixing the "lines" variable exception. That's probably all I'm doing for today:
Full fixed files available here if needed, I'll try setting up a fork on github instead later to make that more convenient: https://sourceforge.net/p/vim-tohtml/code/ci/4bb1c61546f0c610dd6981154cdfe23f76b452f1/tree/syntax/2html.vim
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- a1e2b18 fix: scope problem
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.