fix(help): :helptags regressions

Problem: Parent commit regressed some behavior of the old C helptags-gen impl: - helpfiles in sub-directories are named by basename, so :help fails - "help-tags" always names "tags", never "tags-nl" - E150, E151, E152 and E153 are never reported - existing tags file is not overwritten if no tags were found - a duplicate tag aborts the run with a Lua traceback, skipping the remaining directories and languages - `*.TXT` and `*.FRX` (uppercase) are not recognized as helpfiles - the tree-sitter-vimdoc grammar accepts tags that the C parser rejected: `*a|b*` (breaks |links|) and unterminated "*tag" Solution: - Restore old behavior: tags generated for runtime/doc are now identical to those from the C implementation. Errors are non-fatal messages instead of exceptions, so all directories are still processed. - Fail the build if generating helptags reports any `v:errmsg`. - `:helptags ALL` now reports E152 for "doc" directories it cannot write, instead of silently skipping them.
2026-08-26 09:01:57 +00:00 · 2026-08-24 22:07:53 +02:00
parent c1e4815bff
commit 20edb8fa9a
4 changed files with 120 additions and 56 deletions
--- a/runtime/CMakeLists.txt
+++ b/runtime/CMakeLists.txt
@@ -41,12 +41,14 @@ foreach(PACKAGE ${PACKAGES})
    add_custom_command(OUTPUT "${GENERATED_PACKAGE_DIR}/${PACKNAME}/doc/tags"
      COMMAND ${CMAKE_COMMAND} -E copy_directory
        ${PACKAGE} ${GENERATED_PACKAGE_DIR}/${PACKNAME}
+      # "-c :helptags" (E154 duplicate tags, …) does not set the exit code, so check v:errmsg.
      COMMAND ${CMAKE_COMMAND} -E env "VIMRUNTIME=${PROJECT_SOURCE_DIR}/runtime" ${NVIM_HOST_PRG}
-        -u NONE -i NONE -e --headless -c "helptags doc" -c quit
+        -u NONE -i NONE -e --headless -c "helptags doc" -c "exe 'cquit' !empty(v:errmsg)"
      DEPENDS
        nvim_bin
        nvim_runtime_deps
      WORKING_DIRECTORY "${GENERATED_PACKAGE_DIR}/${PACKNAME}"
+      VERBATIM
    )

    set("${PACKNAME}_DOC_NAMES")
@@ -73,12 +75,14 @@ add_custom_command(OUTPUT ${GENERATED_HELP_TAGS}
  COMMAND ${CMAKE_COMMAND} -E remove_directory doc
  COMMAND ${CMAKE_COMMAND} -E copy_directory
    ${PROJECT_SOURCE_DIR}/runtime/doc doc
+  # "-c :helptags" (E154 duplicate tags, …) does not set the exit code, so check v:errmsg.
  COMMAND ${CMAKE_COMMAND} -E env "VIMRUNTIME=${PROJECT_SOURCE_DIR}/runtime" ${NVIM_HOST_PRG}
-    -u NONE -i NONE -e --headless -c "helptags ++t doc" -c quit
+    -u NONE -i NONE -e --headless -c "helptags ++t doc" -c "exe 'cquit' !empty(v:errmsg)"
  DEPENDS
    nvim_bin
    nvim_runtime_deps
  WORKING_DIRECTORY "${GENERATED_RUNTIME_DIR}"
+  VERBATIM
 )

 add_custom_target(
--- a/runtime/doc/deprecated.txt
+++ b/runtime/doc/deprecated.txt
@@ -97,7 +97,7 @@ LUA
 UI

 • *hit-enter* *hit-enter-prompt*	With |ui2|, the legacy "Press ENTER"
-  *press-enter				prompt is never triggered.
+  *press-enter*				prompt is never triggered.

 ------------------------------------------------------------------------------
 DEPRECATED IN 0.11					*deprecated-0.11*
--- a/runtime/lua/vim/_core/help.lua
+++ b/runtime/lua/vim/_core/help.lua
@@ -1,6 +1,7 @@
 local M = {}

 local ts = vim.treesitter
+local echo_err = require('vim._core.util').echo_err

 local tag_exceptions = {
  -- Interpret asterisk (star, '*') literal but name it 'star'
@@ -397,37 +398,45 @@ function M.local_additions()
  end
 end

-local query = ts.query.parse('vimdoc', '(tag (word) @tagname)')
-
 --- @alias Tag { [1]: string, [2]: string, [3]: string} tuple of tag, file, and search command

---Find and report duplicate tags.
---@param tags Tag[]
---@return boolean
-local function find_duplicates(tags)
-  local prevtag, prevfn, has_duplicates = '', '', false
+---Language code of a help file: "en" for "*.txt", "nl" for "*.nlx". See |help-translated|.
+---@param file string
+---@return string?
+local function helpfile_lang(file)
+  local ext = file:sub(-4):lower()
+  return ext == '.txt' and 'en' or ext:match('^%.(%a%a)x$')
+end
+
+---Report duplicate tags (as errmsg, not exception-throwing error).
+---@param tags Tag[] sorted by tag name
+local function report_duplicates(tags)
+  local prevtag, prevfn = '', ''

  for _, tagline in ipairs(tags) do
    local curtag, curfn, _ = unpack(tagline)
    if curtag == prevtag then
-      has_duplicates = true
      local filenames = prevfn ~= curfn and (curfn .. ' and ' .. prevfn) or curfn
-      local msg = ('E154: Duplicate tag "%s" in %s'):format(curtag, filenames)
-      vim.api.nvim_echo({ { msg } }, false, { err = true })
+      echo_err(('E154: Duplicate tag "%s" in %s'):format(curtag, filenames))
    end
    prevtag = curtag
    prevfn = curfn
  end
-
-  return has_duplicates
 end

 ---Extract tags from {file} and add to list of tags. Modifies {tags}.
 ---@param tags Tag[]
 ---@param file string
-local function extract_tags(tags, file)
-  local filename = vim.fs.basename(file)
-  local source = vim.fn.readblob(file)
+---@param name string Path of {file} relative to the help directory, as stored in the tags file.
+local function extract_tags(tags, file, name)
+  local ok, source = pcall(vim.fn.readblob, file)
+  if not ok then
+    echo_err(('E153: Unable to open %s for reading'):format(file))
+    return
+  end
+  --- @cast source string
+
+  local query = ts.query.parse('vimdoc', '(tag (word) @tagname)')
  local parser = ts.get_string_parser(source, 'vimdoc')

  local tree = assert(parser:parse())
@@ -435,16 +444,23 @@ local function extract_tags(tags, file)
  for _, match in query:iter_matches(root, source) do
    for id, node in pairs(match) do
      if query.captures[id] == 'tagname' then
-        -- Only accept a *tag* when there is white space (or nothing) before it
-        -- and it is followed by a white character or end-of-line.
-        local _, _, start_byte, _, _, end_byte = node[1]:parent():range(true)
+        -- Only accept a *tag* when it is closed, has no "|" (which would break |links|), there is
+        -- whitespace (or nothing) before it, and followed by whitespace or end-of-line.
+        local tag_node = assert(node[1]:parent())
+        local _, _, start_byte = tag_node:start()
+        local _, _, end_byte = tag_node:end_()
        local before = source:sub(start_byte, start_byte)
        local after = source:sub(end_byte + 1, end_byte + 1)
-        if before:match('^[ \t\n\r]?$') and after:match('^[ \t\n\r]?$') then
-          local tagname = ts.get_node_text(node[1], source)
+        local tagname = ts.get_node_text(node[1], source)
+        if
+          source:sub(end_byte, end_byte) == '*'
+          and not tagname:find('|', 1, true)
+          and before:match('^[ \t\n\r]?$')
+          and after:match('^[ \t\n\r]?$')
+        then
          local escaped = tagname:gsub('[\\/]', '\\%0')
          local searchcmd = '/*' .. escaped .. '*'
-          table.insert(tags, { tagname, filename, searchcmd })
+          table.insert(tags, { tagname, name, searchcmd })
        end
      end
    end
@@ -453,23 +469,20 @@ end

 --- Extract tags from helpfiles and combine in a single 'tags' file.
 --- @param helpfiles string[] list of helpfiles
+--- @param dir string Help directory; tag entries name the helpfiles relative to it.
 --- @param outpath string path to write the 'tags' file to.
 --- @param include_helptags_tag boolean true if the 'help-tags' tag should be included
-local function gen_tagsfile(helpfiles, outpath, include_helptags_tag)
+local function gen_tagsfile(helpfiles, dir, outpath, include_helptags_tag)
  ---@type Tag[]
  local tags = {}

  -- (1) extract tags from all files
  for _, file in ipairs(helpfiles) do
-    extract_tags(tags, file)
+    extract_tags(tags, file, vim.fs.relpath(dir, file) or vim.fs.basename(file))
  end

  if include_helptags_tag then
-    table.insert(tags, { 'help-tags', 'tags', '1' })
-  end
-
-  if vim.tbl_isempty(tags) then
-    return
+    table.insert(tags, { 'help-tags', vim.fs.basename(outpath), '1' })
  end

  -- (2) sort alphabetically on tag name
@@ -477,20 +490,19 @@ local function gen_tagsfile(helpfiles, outpath, include_helptags_tag)
    return a[1] < b[1]
  end)

-  -- (3) check duplicates
-  local has_duplicates = find_duplicates(tags)
+  -- (3) report duplicates (non-fatal errmsg: the tags file is still written)
+  report_duplicates(tags)

  -- (4) write tags to file
-  local f = assert(io.open(outpath, 'w'))
+  local f = io.open(outpath, 'w')
+  if not f then
+    echo_err(('E152: Cannot open %s for writing'):format(outpath))
+    return
+  end
  for _, tag in ipairs(tags) do
    f:write(table.concat(tag, '\t') .. '\n')
  end
  f:close()
-
-  -- tags file has to be written before we can error
-  if has_duplicates then
-    error('duplicate tags')
-  end
 end

 --- Create a "tags" file for all help files in the given directory.
@@ -511,33 +523,34 @@ function M.gen_tags(dir, include_index_tag)
  local dirs = dir and { vim.fs.normalize(dir) } or vim.api.nvim_get_runtime_file('doc', true)
  local vimruntime = vim.fs.normalize(vim.fs.joinpath(vim.env.VIMRUNTIME, 'doc'))

+  if dir and vim.fn.isdirectory(dirs[1]) == 0 then
+    echo_err(('E150: Not a directory: %s'):format(dir))
+    return
+  end
+
  for _, directory in ipairs(dirs) do
    local files = vim.fs.find(function(name, _)
-      return vim.endswith(name, '.txt')
+      return helpfile_lang(name) ~= nil
    end, { path = directory, type = 'file', limit = math.huge })

-    local outpath = vim.fs.joinpath(directory, 'tags')
-    gen_tagsfile(files, outpath, include_index_tag or directory == vimruntime)
+    if vim.tbl_isempty(files) then
+      echo_err(('E151: No match: %s'):format(vim.fs.joinpath(directory, '**/*.txt')))
+    end

-    -- handle translated help files per language
-    local translated = vim.fs.find(function(name, _)
-      -- "*.[a-z][a-z]x", see :help help-translated
-      return name:match('%.%l%lx', -4)
-    end, { path = directory, type = 'file', limit = math.huge })
-
-    -- categorize translated files per two-letter language code
+    -- categorize helpfiles per language, see |help-translated|
    ---@type table<string, string[]>
    local per_lang = {}
-    for _, file in ipairs(translated) do
-      -- extract language code "nl" from filename "plugin.nlx"
-      local lang = file:sub(-3, -2)
+    for _, file in ipairs(files) do
+      local lang = assert(helpfile_lang(file))
      per_lang[lang] = per_lang[lang] or {}
      table.insert(per_lang[lang], file)
    end

    for lang, langfiles in pairs(per_lang) do
-      local tagsfile = vim.fs.joinpath(directory, 'tags-' .. lang)
-      gen_tagsfile(langfiles, tagsfile, include_index_tag or directory == vimruntime)
+      -- English is an exception: "*.txt" files generate the "tags" file.
+      local tagsfile = lang == 'en' and 'tags' or ('tags-%s'):format(lang)
+      local outpath = vim.fs.joinpath(directory, tagsfile)
+      gen_tagsfile(langfiles, directory, outpath, include_index_tag or directory == vimruntime)
    end
  end
 end
--- a/test/functional/ex_cmds/help_spec.lua
+++ b/test/functional/ex_cmds/help_spec.lua
@@ -350,12 +350,43 @@ describe(':helptags', function()
  end)

  it('{dir}', function()
+    -- Helpfiles in sub-directories are found and named relative to {dir}.
+    fn.mkdir('Xhelptags/doc/sub', 'p')
+    -- A "|" in a tag would break |links|, and "*Xd" is not closed, so neither is a tag.
+    write_file('Xhelptags/doc/sub/Xc.txt', '*Xc*\n*X|c*\n*Xd\n')
+
    command('helptags Xhelptags/doc')

-    eq(eval("['Xa	Xa.txt	/*Xa*','Xb	Xb.txt	/*Xb*']"), eval("readfile('Xhelptags/doc/tags')"))
+    eq(
+      eval("['Xa	Xa.txt	/*Xa*','Xb	Xb.txt	/*Xb*','Xc	sub/Xc.txt	/*Xc*']"),
+      eval("readfile('Xhelptags/doc/tags')")
+    )

    command('help Xa')
    eq('*Xa*', api.nvim_get_current_line())
+
+    command('help Xc')
+    eq('*Xc*', api.nvim_get_current_line())
+  end)
+
+  it('overwrites existing tags file', function()
+    write_file('Xhelptags/doc/tags', 'Xstale	Xold.txt	/*Xstale*', nil, true)
+    write_file('Xhelptags/doc/Xa.txt', 'no tags here', nil, true)
+    write_file('Xhelptags/doc/Xb.txt', 'no tags here', nil, true)
+
+    command('helptags Xhelptags/doc')
+    eq({}, eval("readfile('Xhelptags/doc/tags')"))
+  end)
+
+  it('reports E150 E151 E152', function()
+    eq(true, t.pcall_err(command, 'helptags Xhelptags/doc/Xa.txt'):find('E150') ~= nil)
+
+    fn.mkdir('Xhelptags/Xempty', 'p')
+    eq(true, t.pcall_err(command, 'helptags Xhelptags/Xempty'):find('E151') ~= nil)
+
+    -- A directory named "tags" cannot be opened for writing.
+    fn.mkdir('Xhelptags/doc/tags', 'p')
+    eq(true, t.pcall_err(command, 'helptags Xhelptags/doc'):find('E152') ~= nil)
  end)

  it('ALL', function()
@@ -369,8 +400,12 @@ describe(':helptags', function()
  end)

  it('++t', function()
+    write_file('Xhelptags/doc/Xa.nlx', '*Xa*', nil, true)
+
    command('helptags ++t Xhelptags/doc')
    eq('help-tags	tags	1', eval("readfile('Xhelptags/doc/tags')[-1]"))
+    -- Each language gets a "help-tags" tag naming its own tags file.
+    eq('help-tags	tags-nl	1', eval("readfile('Xhelptags/doc/tags-nl')[-1]"))
  end)

  it('generates help-tag tag for VIMRUNTIME', function()
@@ -397,11 +432,23 @@ describe(':helptags', function()
    eq(true, msg:find('E154') ~= nil)

    eq(1, eval("filereadable('Xhelptags/doc/tags')"))
+
+    -- Duplicates do not abort the run: other languages are still processed.
+    write_file('Xhelptags/doc/Xa.nlx', '*Xnl*', nil, true)
+
+    msg = t.pcall_err(command, 'helptags Xhelptags/doc')
+    eq(true, msg:find('E154') ~= nil)
+    eq(false, msg:find('E5108') ~= nil)
+    eq(1, eval("filereadable('Xhelptags/doc/tags-nl')"))
  end)

  it('with translated help files', function()
    write_file('Xhelptags/doc/Xa.nlx', '*Xa*', nil, true)
+    -- The suffix is matched case-insensitively.
+    write_file('Xhelptags/doc/Xc.FRX', '*Xc*', nil, true)
+
    command('helptags Xhelptags/doc')
-    eq(1, eval("filereadable('Xhelptags/doc/tags-nl')"))
+    eq(eval("['Xa	Xa.nlx	/*Xa*']"), eval("readfile('Xhelptags/doc/tags-nl')"))
+    eq(eval("['Xc	Xc.FRX	/*Xc*']"), eval("readfile('Xhelptags/doc/tags-fr')"))
  end)
 end)