[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.
fritzophrenic
> - let s:wrapperfunc_lines[-1] =<< trim ENDLET
- let l:saved_style = get(s:stylelist,a:style_id)
- if type(l:saved_style) == type(0)
- unlet l:saved_style
- let l:saved_style = s:CSS1(a:style_id)
- if l:saved_style != ""
- let l:saved_style = "." .. l:style_name .. " { " .. l:saved_style .. "}"
- endif
- let s:stylelist[a:style_id] = l:saved_style
- endif
+ # get primary style info from cache or build it on the fly if not found
+ trim_tmp =<< trim ENDLET
+ saved_style = get(stylelist, style_id, () => {
+ var res = CSS1(style_id)
+ if !empty(res)
+ saved_style = "." .. style_name .. " { " .. saved_style .. "}"
⬇️ Suggested change
- saved_style = "." .. style_name .. " { " .. saved_style .. "}"
+ res = "." .. style_name .. " { " .. res .. "}"
Fixes CSS validation errors (and corresponding broken style definitions).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> +var foldfillchar: string
+if has('folding') && !settings.ignore_folding
+ foldfillchar = &fillchars[charidx(&fillchars, matchend(&fillchars, 'fold:'))]
+ if foldfillchar == ''
+ foldfillchar = '-'
endif
endif
-let s:difffillchar = &fillchars[matchend(&fillchars, 'diff:')]
-if s:difffillchar == ''
- let s:difffillchar = '-'
+var difffillchar = &fillchars[charidx(&fillchars, matchend(&fillchars, 'diff:'))]
+if difffillchar == ''
+ difffillchar = '-'
endif
⬇️ Suggested change
-var foldfillchar: string
-if has('folding') && !settings.ignore_folding
- foldfillchar = &fillchars[charidx(&fillchars, matchend(&fillchars, 'fold:'))]
- if foldfillchar == ''
- foldfillchar = '-'
- endif
-endif
-let s:difffillchar = &fillchars[matchend(&fillchars, 'diff:')]
-if s:difffillchar == ''
- let s:difffillchar = '-'
-var difffillchar = &fillchars[charidx(&fillchars, matchend(&fillchars, 'diff:'))]
-if difffillchar == ''
- difffillchar = '-'
-endif
+var foldfillchar = ''
+var charidx: number
+if has('folding') && !settings.ignore_folding
+ charidx = matchend(&fillchars, 'fold:')
+ if charidx >= 0
+ foldfillchar = &fillchars[charidx(&fillchars, charidx)]
+ endif
+ if foldfillchar == ''
+ foldfillchar = '-'
+ endif
+endif
+var difffillchar = ''
+charidx = matchend(&fillchars, 'diff:')
+if charidx >= 0
+ difffillchar = &fillchars[charidx(&fillchars, charidx)]
+endif
+if difffillchar == ''
+ difffillchar = '-'
+endif
Fixes wrong character used for diff filler. Also fixes similar theoretical issue with foldtext filler.
The issue was, when matchend() finds no matches for "diff:" (or "fold:"), it will return -1. This was causing 2html to default to the last character in 'fillchars' instead of defaulting to "-".
—
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)After my last two fixes, there is only one remaining test failure:
From /home/ben/code/vim-tohtml/testdir/test_hi_link.vim:
Found errors in Test_direct_hi_rules():
command line..script /home/ben/code/vim-src/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_direct_hi_rules[4]..8_check_direct_styles[8]..8_check_style_rule line 3: Extra CSS rule for LineNr: Expected False but got 21
It looks like this failure comes from a duplicate LineNr style definition being added for some reason. Here's the options line from the generated HTML in case it's relevant:
<meta name="settings" content="number_lines,dynamic_folds,use_css,diff_one_file,expand_tabs,prevent_copy=,use_input_for_pc=none">
—
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.
- 28522fc Update runtime/syntax/2html.vim
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining commented on this pull request.
> +var foldfillchar: string
+if has('folding') && !settings.ignore_folding
+ foldfillchar = &fillchars[charidx(&fillchars, matchend(&fillchars, 'fold:'))]
+ if foldfillchar == ''
+ foldfillchar = '-'
endif
endif
-let s:difffillchar = &fillchars[matchend(&fillchars, 'diff:')]
-if s:difffillchar == ''
- let s:difffillchar = '-'
+var difffillchar = &fillchars[charidx(&fillchars, matchend(&fillchars, 'diff:'))]
+if difffillchar == ''
+ difffillchar = '-'
endif
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.
- b0681d4 fix: fillchar when empty
—
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)From /home/ben/code/vim-tohtml/testdir/test_hi_link.vim:
Found errors in Test_direct_hi_rules():
command line..script /home/ben/code/vim-src/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_direct_hi_rules[4]..8_check_direct_styles[8]..8_check_style_rule line 3: Extra CSS rule for LineNr: Expected False but got 21
This failure appears to be caused by a duplicate LineNr style definition being added for some reason. Here's the options line from the generated HTML in case it's relevant:
<meta name="settings" content="number_lines,dynamic_folds,use_css,diff_one_file,expand_tabs,prevent_copy=,use_input_for_pc=none">
Maybe the issue is in this line:
call add(s:lines, '<meta name="settings" content="'.. \ join(filter(keys(s:settings),'s:settings[v:val]'),',').. \ ',prevent_copy='..s:settings.prevent_copy.. \ ',use_input_for_pc='..s:settings.use_input_for_pc.. \ '"'..s:tag_close)
which I've converted to:
add(lines, '<meta name="settings" content="' .. join(filter(keys(settings), (_, k) => { var v = settings[k] if type(v) == v:t_number return v != 0 elseif type(v) == v:t_bool return !empty(v) else return false endif }), ',') .. ',prevent_copy=' .. settings.prevent_copy .. ',use_input_for_pc=' .. settings.use_input_for_pc .. '"' .. tag_close)
But I haven't figured out what went wrong yet.
—
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.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 21 commits.
- f99850a runtime(tohtml): convert tohtml plugin to Vim9 script
- 1910b1a perf: use def to compile heat code to bytecode
- 758a6f3 perf: use def to compile into bytecode
- 35a13c8 fix: can't show b_color
- 417af31 fix: correct synIDattr boolean checks and progress bar logic
- 46d0139 fix(tohtml): fix variable scope and initialization in line loop
- a2cf3fd change document
- cf58177 Update Changelog
- 59a7825 fix some problem, but diff is still mal-functions
- af105d6 fix ':' before %
- 2854b79 fix: diffstyle
- d2f885b revert changes for Add_diff_fill
- ab1d0a2 fix: foldfillchar
- 4f06216 use '..=' istead of 'a = a ..'
- 35e9bb6 fix: type in type(v)
- 700488a remove saved_style_tmp
- fd9c1c8 fix: scope problem
- 6f766c0 Update runtime/syntax/2html.vim
- fc11e8a fix: fillchar when empty
- 8ff527c Update MAINTAINERS
- 4f9c90b Update version9.txt
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)Oh, the meta line is fine as far as I know (I suppose I should check if I have a test for that). I was only providing the meta line so you could see what options are set by the test, to reproduce the issue on your end if you want to debug.
The actual issue is there are two duplicate CSS lines generated to define the LineNr highlight. Since I have a specific test for that, I assume there was a bugfix at some point to remove a duplicate LineNr highlight definition which seems to have regressed.
—
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)@chrisbra by the way, are you interested in adding the tohtml tests to the automated checks at all? I'm not familiar with how those are configured or anything but would be willing to learn and put together a pull request. They do depend on an external HTML validator tool, which currently is a vnu.jar java file needing a JRE installed, in case that affects the feasibility of adding as an automated test.
—
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'm still considering the performance issue. It seems we've used too much string concatenation. Would it be better to use a list instead?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19915)@chrisbra by the way, are you interested in adding the tohtml tests to the automated checks at all?
Yes, I think this would make sense if it is not too heavy on what needs to be installed for this, if only for having tests available and you are not the single one knowing how to validate it does not regress.
—
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.
- 1381c5a use syn_cache and conceal_cache
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19915)What is the status here?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)The last I knew there was still the one test failure, and I think @mao-yining was looking into a performance issue? I'm not clear what that was.
@mao-yining , did this branch achieve a speedup already and you're just trying to wring even more performance out of it? Or is it actually not faster than before?
I have busy with a big home improvement project the last couple weeks, but will find some time soon to debug that last test failure if someone else doesn't beat me to it. I'd also like to look at the diff in more detail. But I think I'm inclined to merge once that is done. I don't know when I'll be able to get the test suite expanded and updated for use as a GitHub action. I'm sure any bugs not found by current tests will surface eventually.
I do have one concern: I think switching to vim9script breaks compatibility with Neovim, and I think at the moment they still pull this script in. Is there someone from that project we should tag for visibility?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19915)I don't follow Neovim closely, but I seem to remember that they dropped 2html.vim for a lua based solution.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
zeertzjq
zeertzjq left a comment (vim/vim#19915)Yes it was removed in neovim/neovim#27097
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 0 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 2e2652c Update version9.txt
—
View it on GitHub or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)@mao-yining , did this branch achieve a speedup already and you're just trying to wring even more performance out of it? Or is it actually not faster than before?
It is faster than before — just didn't meet my expectations.
Because Neovim's :TOhtml is about 10 times faster than this version. I profiled it, and currently the most costly part is synID().
Anyway, I think this is fine as long as all tests pass.
—
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)Additionally, there is currently no support for textprop. I think you could consider adding this feature in the future. I've submitted an issue in the repository https://sourceforge.net/p/vim-tohtml/issues/44/.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19915)is it still breaking a test?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#19915)@fritzophrenic I can't reproduce the LineNr test fail on my computer. Now when I run the tests, they also fail, but the legacy version also fails on my computer with exactly the same error message.
If you have time to review, please use GitHub's approve feature to clearly indicate your approval.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
fritzophrenic left a comment (vim/vim#19915)I'll find time to review this weekend. In the meantime, I verified the failing test did work prior to the vim9script conversion but was failing after. However, I also found the issue and am adding a suggested fix.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
@fritzophrenic commented on this pull request.
>
-" caches of style data
-" initialize to include line numbers if using them
-if s:settings.number_lines
- let s:stylelist = { s:LINENR_ID : ".LineNr { " .. s:CSS1( s:LINENR_ID ) .. "}" }
+# caches of style data
+# initialize to include line numbers if using them
+var stylelist: dict<string>
+if settings.number_lines
+ stylelist = { LINENR_ID: ".LineNr { " .. CSS1(LINENR_ID) .. "}" }
Fix duplicated LineNr highlight rule
Vim9script changes the way keys are parsed in literal dictionary definitions.
Keys conisting only of alphanumeric characters, underscore, and dash are
interpreted literally instead of as a variable value or other expression. To
force the LINENR_ID variable to be used as a variable instead of inserting a
new dictionary value with a key of the literal string "LINENR_ID", we need to
surround with square brackets. See vim9-literal-dict.
- stylelist = { LINENR_ID: ".LineNr { " .. CSS1(LINENR_ID) .. "}" }
+ stylelist = { [LINENR_ID]: ".LineNr { " .. CSS1(LINENR_ID) .. "}" }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 42a628c fix LineNr in stylelist
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
@fritzophrenic commented on this pull request.
In runtime/autoload/tohtml.vim:
> - if a:line2 >= a:line1 - let g:html_start_line = a:line1 - let g:html_end_line = a:line2 +vim9script +# Vim autoload file for the tohtml plugin. +# Maintainer: Ben Fritz <fritzo...@gmail.com> +# Last Change: 2026 Apr 4 +# +# Additional contributors: +# +# Original by Bram Moolenaar <Br...@vim.org> +# Diff2HTML() added by Christian Brabandt <c...@256bit.org> +# +# See Mercurial change logs for more! + +# this file uses line continuations (but in vim9script we avoid them)
There are no line continuations in this file anymore, delete comment about them.
⬇️ Suggested change-# this file uses line continuations (but in vim9script we avoid them)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> endif
- runtime syntax/2html.vim "}}}
- else "{{{
- let win_list = []
- let buf_list = []
- windo if &diff | call add(win_list, winbufnr(0)) | endif
- let s:settings.whole_filler = 1
- let g:html_diff_win_num = 0
+ runtime syntax/2html.vim #}}}
+ else #{{{
+ var win_list = range(1, winnr('$'))->mapnew((_, w) => winbufnr(w))
This change is not equivalent to the previous behavior and inconsistent with the help text. Previously, if you have multiple windows open and only some of them are part of a diff, only the diffed windows are added to the TOhtml output. With this new code, all windows are added to the TOhtml output. The win_list code needs updated to restore the old behavior.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> endfor
unlet g:html_diff_win_num
- call tohtml#Diff2HTML(win_list, buf_list)
- endif "}}}
-
+ Diff2HTML(win_list, buf_list)
+ endif
⬇️ Suggested change
- endif + endif #}}}
Restore end fold marker.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> + 'iso-10646-ucs-2': 'ucs-2', + 'csunicode': 'ucs-2', + 'utf-16': 'utf-16', + 'utf-16be': 'utf-16', + 'utf-16le': 'utf-16le', + 'utf-32': 'ucs-4', + 'utf-32be': 'ucs-4', + 'utf-32le': 'ucs-4le', + 'iso-10646-ucs-4': 'ucs-4', + 'csucs4': 'ucs-4' +} +#}}} + +var settings: dict<any> + +export def Convert2HTML(line1: number, line2: number)
Original file had fold markers to make it easier to navigate/overview the file. Let's put those back.
⬇️ Suggested change-export def Convert2HTML(line1: number, line2: number)
+export def Convert2HTML(line1: number, line2: number) #{{{
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
> unlet g:html_start_line unlet g:html_end_line - unlet s:settings -endfunc "}}} + settings = null_dict +enddef⬇️ Suggested change
-enddef +enddef #}}}
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
fritzophrenic
>
-func! tohtml#Diff2HTML(win_list, buf_list) "{{{
- let xml_line = ""
- let tag_close = '>'
+export def Diff2HTML(win_list: list<number>, buf_list: list<number>)
⬇️ Suggested change
-export def Diff2HTML(win_list: list<number>, buf_list: list<number>)
+export def Diff2HTML(win_list: list<number>, buf_list: list<number>) #{{{
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining pushed 1 commit.
- 48ea13f Response for review
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mao-Yining
@mao-yining commented on this pull request.
In runtime/autoload/tohtml.vim:
> endif
- runtime syntax/2html.vim "}}}
- else "{{{
- let win_list = []
- let buf_list = []
- windo if &diff | call add(win_list, winbufnr(0)) | endif
- let s:settings.whole_filler = 1
- let g:html_diff_win_num = 0
+ runtime syntax/2html.vim #}}}
+ else #{{{
+ var win_list = range(1, winnr('$'))->mapnew((_, w) => winbufnr(w))
Thanks.
—
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)FYI I'm slowly working through the review a little bit at a time. It did look like a few more places where comments or fold markers went missing, if you wanted to proactively check and fix those. I'll try to finish reviewing over next few days (and write a test for checking which windows are included in the output for multi-window diff mode).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.