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.

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(

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*

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.
+---Language code of a help file: "en" for "*.txt", "nl" for "*.nlx". See |help-translated|.
----@param tags Tag[]
+---@param file string
----@return boolean
+---@return string?
-local function find_duplicates(tags)
+local function helpfile_lang(file)
-  local prevtag, prevfn, has_duplicates = '', '', false
+  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)
+      echo_err(('E154: Duplicate tag "%s" in %s'):format(curtag, filenames))
-      vim.api.nvim_echo({ { msg } }, false, { err = true })
     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)
+---@param name string Path of {file} relative to the help directory, as stored in the tags file.
-  local filename = vim.fs.basename(file)
+local function extract_tags(tags, file, name)
-  local source = vim.fn.readblob(file)
+  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
+        -- Only accept a *tag* when it is closed, has no "|" (which would break |links|), there is
-        -- and it is followed by a white character or end-of-line.
+        -- whitespace (or nothing) before it, and followed by whitespace or end-of-line.
-        local _, _, start_byte, _, _, end_byte = node[1]:parent():range(true)
+        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' })
+    table.insert(tags, { 'help-tags', vim.fs.basename(outpath), '1' })
-  end
-
-  if vim.tbl_isempty(tags) then
-    return
   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
+  -- (3) report duplicates (non-fatal errmsg: the tags file is still written)
-  local has_duplicates = find_duplicates(tags)
+  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')
+    if vim.tbl_isempty(files) then
-    gen_tagsfile(files, outpath, include_index_tag or directory == vimruntime)
+      echo_err(('E151: No match: %s'):format(vim.fs.joinpath(directory, '**/*.txt')))
+    end
 
-    -- handle translated help files per language
+    -- categorize helpfiles per language, see |help-translated|
-    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
     ---@type table<string, string[]>
     local per_lang = {}
-    for _, file in ipairs(translated) do
+    for _, file in ipairs(files) do
-      -- extract language code "nl" from filename "plugin.nlx"
+      local lang = assert(helpfile_lang(file))
-      local lang = file:sub(-3, -2)
       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)
+      -- English is an exception: "*.txt" files generate the "tags" file.
-      gen_tagsfile(langfiles, tagsfile, include_index_tag or directory == vimruntime)
+      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

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)