feat(defaults): store spellfile in stdpath('data') #33048

Problem:
First rtp directory is unpredictable and not in line with XDG
base spec.

Solution:
Use stdpath('data')/spell as directory if 'spellfile' is not set.

Co-authored-by: zeertzjq <zeertzjq@outlook.com>
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>

runtime/doc/news.txt

@@ -177,7 +177,8 @@ CHANGED FEATURES                                                 *news-changed*
 
 These existing features changed their behavior.
 
-• todo
+• 'spellfile' location defaults to `stdpath("data").."/spell/"` instead of the
+  first writable directoy in 'runtimepath'.
 
 ==============================================================================
 REMOVED FEATURES                                                 *news-removed*

runtime/doc/options.txt

@@ -5755,11 +5755,10 @@ A jump table for the options with a short description can be found at |Q_op|.
 	It may also be a comma-separated list of names.  A count before the
 	|zg| and |zw| commands can be used to access each.  This allows using
 	a personal word list file and a project word list file.
-	When a word is added while this option is empty Vim will set it for
-	you: Using the first directory in 'runtimepath' that is writable.  If
-	there is no "spell" directory yet it will be created.  For the file
-	name the first language name that appears in 'spelllang' is used,
-	ignoring the region.
+	When a word is added while this option is empty Nvim will use
+	(and auto-create) `stdpath('data')/spell/`. For the file name the
+	first language name that appears in 'spelllang' is used, ignoring the
+	region.
 	The resulting ".spl" file will be used for spell checking, it does not
 	have to appear in 'spelllang'.
 	Normally one file is used for all regions, but you can add the region

runtime/doc/vim_diff.txt

@@ -79,6 +79,7 @@ Defaults                                            *defaults* *nvim-defaults*
 - 'showcmd' is enabled
 - 'sidescroll' defaults to 1
 - 'smarttab' is enabled
+- 'spellfile' defaults to `stdpath("data").."/spell/"`
 - 'startofline' is disabled
 - 'switchbuf' defaults to "uselast"
 - 'tabpagemax' defaults to 50

runtime/lua/vim/_meta/options.lua

@@ -6123,11 +6123,10 @@ vim.bo.spc = vim.bo.spellcapcheck
 --- It may also be a comma-separated list of names.  A count before the
 --- `zg` and `zw` commands can be used to access each.  This allows using
 --- a personal word list file and a project word list file.
---- When a word is added while this option is empty Vim will set it for
---- you: Using the first directory in 'runtimepath' that is writable.  If
---- there is no "spell" directory yet it will be created.  For the file
---- name the first language name that appears in 'spelllang' is used,
---- ignoring the region.
+--- When a word is added while this option is empty Nvim will use
+--- (and auto-create) `stdpath('data')/spell/`. For the file name the
+--- first language name that appears in 'spelllang' is used, ignoring the
+--- region.
 --- The resulting ".spl" file will be used for spell checking, it does not
 --- have to appear in 'spelllang'.
 --- Normally one file is used for all regions, but you can add the region

src/nvim/options.lua

@@ -8158,11 +8158,10 @@ local options = {
         It may also be a comma-separated list of names.  A count before the
         |zg| and |zw| commands can be used to access each.  This allows using
         a personal word list file and a project word list file.
-        When a word is added while this option is empty Vim will set it for
-        you: Using the first directory in 'runtimepath' that is writable.  If
-        there is no "spell" directory yet it will be created.  For the file
-        name the first language name that appears in 'spelllang' is used,
-        ignoring the region.
+        When a word is added while this option is empty Nvim will use
+        (and auto-create) `stdpath('data')/spell/`. For the file name the
+        first language name that appears in 'spelllang' is used, ignoring the
+        region.
         The resulting ".spl" file will be used for spell checking, it does not
         have to appear in 'spelllang'.
         Normally one file is used for all regions, but you can add the region

src/nvim/spellfile.c

@@ -262,6 +262,7 @@
 #include "nvim/os/input.h"
 #include "nvim/os/os.h"
 #include "nvim/os/os_defs.h"
+#include "nvim/os/stdpaths_defs.h"
 #include "nvim/os/time.h"
 #include "nvim/os/time_defs.h"
 #include "nvim/path.h"
@@ -5547,9 +5548,11 @@ void spell_add_word(char *word, int len, SpellAddType what, int idx, bool undo)
 }
 
 // Initialize 'spellfile' for the current buffer.
+//
+// If the location does not exist, create it. Defaults to
+// stdpath("data") + "/spell/{spelllang}.{encoding}.add".
 static void init_spellfile(void)
 {
-  int l;
   char *lend;
   bool aspath = false;
   char *lstart = curbuf->b_s.b_p_spl;
@@ -5558,8 +5561,6 @@ static void init_spellfile(void)
     return;
   }
 
-  char *buf = xmalloc(MAXPATHL);
-
   // Find the end of the language name.  Exclude the region.  If there
   // is a path separator remember the start of the tail.
   for (lend = curwin->w_s->b_p_spl; *lend != NUL
@@ -5570,49 +5571,40 @@ static void init_spellfile(void)
     }
   }
 
-  // Loop over all entries in 'runtimepath'.  Use the first one where we
-  // are allowed to write.
-  char *rtp = p_rtp;
-  while (*rtp != NUL) {
-    if (aspath) {
-      // Use directory of an entry with path, e.g., for
-      // "/dir/lg.utf-8.spl" use "/dir".
-      xmemcpyz(buf, curbuf->b_s.b_p_spl, (size_t)(lstart - curbuf->b_s.b_p_spl - 1));
-    } else {
-      // Copy the path from 'runtimepath' to buf[].
-      copy_option_part(&rtp, buf, MAXPATHL, ",");
-    }
-    if (os_file_is_writable(buf) == 2) {
-      // Use the first language name from 'spelllang' and the
-      // encoding used in the first loaded .spl file.
-      if (aspath) {
-        xmemcpyz(buf, curbuf->b_s.b_p_spl, (size_t)(lend - curbuf->b_s.b_p_spl));
-      } else {
-        // Create the "spell" directory if it doesn't exist yet.
-        l = (int)strlen(buf);
-        vim_snprintf(buf + l, MAXPATHL - (size_t)l, "/spell");
-        if (os_file_is_writable(buf) != 2) {
-          os_mkdir(buf, 0755);
-        }
+  char *buf = xmalloc(MAXPATHL);
+  size_t buf_len = MAXPATHL;
 
-        l = (int)strlen(buf);
-        vim_snprintf(buf + l, MAXPATHL - (size_t)l,
-                     "/%.*s", (int)(lend - lstart), lstart);
-      }
-      l = (int)strlen(buf);
-      char *fname = LANGP_ENTRY(curwin->w_s->b_langp, 0)
-                    ->lp_slang->sl_fname;
-      vim_snprintf(buf + l, MAXPATHL - (size_t)l, ".%s.add",
-                   ((fname != NULL
-                     && strstr(path_tail(fname), ".ascii.") != NULL)
-                    ? "ascii"
-                    : spell_enc()));
-      set_option_value_give_err(kOptSpellfile, CSTR_AS_OPTVAL(buf), OPT_LOCAL);
-      break;
+  if (!aspath) {
+    char *xdg_path = get_xdg_home(kXDGDataHome);
+    xstrlcpy(buf, xdg_path, buf_len);
+    xfree(xdg_path);
+
+    xstrlcat(buf, "/spell", buf_len);
+
+    char *failed_dir;
+    if (os_mkdir_recurse(buf, 0755, &failed_dir, NULL) != 0) {
+      xfree(buf);
+      xfree(failed_dir);
+      return;
     }
-    aspath = false;
+  } else {
+    if ((size_t)(lend - curbuf->b_s.b_p_spl) >= buf_len) {
+      xfree(buf);
+      return;
+    }
+    xmemcpyz(buf, curbuf->b_s.b_p_spl, (size_t)(lend - curbuf->b_s.b_p_spl));
   }
 
+  // Append spelllang
+  vim_snprintf(buf + strlen(buf), buf_len - strlen(buf), "/%.*s", (int)(lend - lstart), lstart);
+
+  // Append ".ascii.add" or ".{enc}.add"
+  char *fname = LANGP_ENTRY(curwin->w_s->b_langp, 0)->lp_slang->sl_fname;
+  const char *enc_suffix =
+    (fname != NULL && strstr(path_tail(fname), ".ascii.") != NULL) ? "ascii" : spell_enc();
+  vim_snprintf(buf + strlen(buf), buf_len - strlen(buf), ".%s.add", enc_suffix);
+
+  set_option_value_give_err(kOptSpellfile, CSTR_AS_OPTVAL(buf), OPT_LOCAL);
   xfree(buf);
 }
 

test/functional/core/spellfile_spec.lua

@@ -14,7 +14,7 @@ local testdir = 'Xtest-functional-spell-spellfile.d'
 
 describe('spellfile', function()
   before_each(function()
-    clear()
+    clear({ env = { XDG_DATA_HOME = testdir .. '/xdg_data' } })
     rmdir(testdir)
     mkdir(testdir)
     mkdir(testdir .. '/spell')
@@ -117,4 +117,29 @@ describe('spellfile', function()
     local fname = fn.fnamemodify(testdir .. '/spell/spell.add', ':p')
     api.nvim_set_option_value('spellfile', fname, {})
   end)
+
+  describe('default location', function()
+    it("is stdpath('data')/spell/en.utf-8.add", function()
+      n.command('set spell')
+      n.insert('abc')
+      n.feed('zg')
+      eq(
+        t.fix_slashes(fn.stdpath('data') .. '/spell/en.utf-8.add'),
+        t.fix_slashes(api.nvim_get_option_value('spellfile', {}))
+      )
+    end)
+
+    it("is not set if stdpath('data') is not writable", function()
+      n.command('set spell')
+      fn.writefile({ '' }, testdir .. '/xdg_data')
+      n.insert('abc')
+      eq("Vim(normal):E764: Option 'spellfile' is not set", exc_exec('normal! zg'))
+    end)
+
+    it("is not set if 'spelllang' is not set", function()
+      n.command('set spell spelllang=')
+      n.insert('abc')
+      eq("Vim(normal):E764: Option 'spellfile' is not set", exc_exec('normal! zg'))
+    end)
+  end)
 end)

test/old/testdir/test_spellfile.vim

@@ -1123,6 +1123,7 @@ endfunc
 " When 'spellfile' is not set, adding a new good word will automatically set
 " the 'spellfile'
 func Test_init_spellfile()
+  throw 'Skipped: Nvim defaults spellfile to stdpath("data")/spell/'
   let save_rtp = &rtp
   let save_encoding = &encoding
   call mkdir('Xrtp/spell', 'pR')