Nightly no longer supports a path in NVIM_APPNAME

[doctorfree]

Neovim 0.9 introduced NVIM_APPNAME as a way to specify the location of the configuration folder, rather than always using ~/.config/nvim. I maintain a project that installs, initializes, and manages multiple Neovim configurations using this feature.

In version 0.9 it is possible to specify a path like neovim/my-cool-config as the NVIM_APPNAME environment variable value. This is especially nice if you have a lot of Neovim configurations to manage, put them all in a subdirectory of ~/.config.

However, nightly Neovim has removed this capability. With nightly if you use a path for NVIM_APPNAME then you get an error message and nvim exits. The issue where this was decided indicates that NVIM_APPNAME was never intended to support paths, simply and strictly the name of a subdirectory in ~/.config.

I do not know what the history or reasoning or intent was with implementation of this feature. But it was working pretty nicely and now it's not. What reasons do people have for disallowing support of a path in NVIM_APPNAME?

[zeertzjq]

Don't you already see in the issue #23056 that it wasn't working properly?

[doctorfree]

Yes, this is the issue I was referring to above where it is stated that support for a path was never intended.

I guess I never ran into the problem with tempname() when using a path. Or did not notice.

If the reason for disallowing paths is that it is not working. And the not working part is tempname(). Then why not fix that rather than scuttle the whole thing? The issue above indicates "calling the alias in the ~/.local/state directory allows tempname() to return a directory.". Perhaps understanding why might be informative.

[doctorfree]

Update: I cannot reliably reproduce this. It appears tempname() intermittently fails on 0.9.1 with a path as NVIM_APPNAME. It does appear that using a path as NVIM_APPNAME creates an issue with tempname().

Using Neovim 0.9.1 and a configuration in a subdirectory with NVIM_APPNAME set to that path, I am able to successfully call tempname(). This is why I never saw this problem using 0.9 and all configs in a subdir. It wasn't broken on 0.9.

My guess is that some change(s) in nightly broke tempname() with a path used for NVIM_APPNAME.

Neovim version used in test on macOS:

NVIM v0.9.1
Build type: Release
LuaJIT 2.1.0-beta3

[doctorfree]

The issue with tempname() and NVIM_APPNAME paths may be fairly simple to address. In src/nvim/fileio.c the vim_mktempdir() function does the following:

    const char *appname = get_appname();
    xstrlcat(tmp, appname, sizeof(tmp));

where get_appname() is returning the value of NVIM_APPNAME. If that is a path then vim_mktempdir() is returning a path which will not get its components created properly. Replacing any separators (e.g. / on Unix/Linux) with _ would resolve that problem.

But, I didn't open this discussion to fix an issue, I just wanted to know why a feature I was using in 0.9 was curtailed in nightly. The answer thus far is "It wasn't working properly with tempname()". My followup question is "Why not fix tempname()?"

[RoryNesbitt]

If this fix really is so trivial then I would be interested in seeing a PR for it.

[doctorfree]

@RoryNesbitt i do not know the Neovim source code well enough to implement a PR and I am starting a new job this week so will have no time for much of anything other than what they pay me for.

I think swapping out the path separators for an underscore in tempname() would involve either using an existing substring replacement function or modify the vim_mktempdir() function in src/nvim/fileio.c something like:

const char *appname = get_appname();
  
int j = 0;
while (appname[j] != '\0'){
    if (vim_ispathsep(appname[j])) {
        appname[j] = '_';
    }
    j++;
}

xstrlcat(tmp, appname, sizeof(tmp));

I did not check the above code,
it's just off the top of my head to illustrate the simplicity of a possible fix to tempname().

Probably better to use an existing substring replacement function if available.

[justinmk]

NVIM_APPNAME now supports relative (not absolute) paths: #25233

[doctorfree]

Nice work! I am closing this discussion.