nvim-dbee
nvim-dbee copied to clipboard
[Windows]: Create a note creates a set of directories instead of just a single `.sql` file throws error on `mkdir`
OS: Windows 11
Nvim Version:
NVIM v0.10.0-dev-2765+gb8858dddb
Build type: RelWithDebInfo
LuaJIT 2.1.1710088188
Reproducible on Windows via default nvim-dbee
setup.
Creating a global or local note will create the note at the following directory which currently appears to cause two issues.
- I think this is what causes
nvim-dbee
to be unable to find the notes created in a previous session - An error is thrown only when creating a local note stating:
Error executing vim.schedule lua callback: Vim:E739: Cannot create directory C:\Users\bwiliams\AppData\Local\nvim-data/dbee/notes/file_source_C\\Users: file already exists stack traceback: [C]: in function 'mkdir' ...nvim-dbee/lua/dbee/ui/editor/init/lua:272: in function 'namespace_create_note' ...nvim-dbee/lua/dbee/ui/drawer/convert.lua:400: in function 'on_submit' ...nui.nvim/lua/nui/input/init.lua:160: in function <...ata/Local/nvim-data/lazy/nui.nvim/lua/nui/input/init.lua:147>
That error I traced back into lua/dbee/ui/drawer/convert.lua
where I found a current_connection_id
variable that prints out the following value:
-
C:\Users\bwilliams\AppData\Local\nvim-data\dbee\notes\file_source_C\Users\bwilliams\AppData\Local\nvim-data\dbee\persistence.jsonXG9pziUp0B
- Which is the id saved to connection string for that database. I'm not sure if that was intended or if this was another one of those side effects of something that works completely different on Linux compared to windows (I should be able to test that this weekend if I get some free time).
It also may be better instead of using the current_connection_id
to create the dir
for the namespace
instead, we use the name
key we set when we create the database connection that way your notes folder would line up with the name you give that specific database connection?
It may be possible that resolving this issue fixes #105.
hey, the ID with a file path is intended, yes. It also works well on unix, since everything is a valid character for a file name. However, there are a bunch of illegal characters for file names in windows, which is a problem.
One way of addressing these problems is to substitute all of these characters with something else, but I don't really like that approach, because its very error prone.
One idea ghat comes to mind would be to encode the id's as base64 (which contains only legal chars AFAIK). base64 functions are also provided inside nvim api, which is very convenient.
edit: well, it appears that I'm wrong. But maybe still more predictable.
edit: even simpler solution is to only match valid chars (e.g. alphanumerics and a few extras).
Is there any reason in particular as to why current_connection_id
is saved as the path to the persistence.json
file instead of just a random string of chars at vim.fn.stdpath('state') .. /dbee/notes/
?
I'm open to whatever you would prefer to make current_connection_id
using the name
key was just a suggestion since it was already being saved using a different value unique to that connection.
Another issue that I discovered later last night is that vim.fn.mkdir
even with the p
flag does not silently fail on Windows and prevents any additional notes from being created because a dir
with that name already exists.
So, it may be better instead of just letting vim.fn.mkdir
silently fail even on Linux that we just check if the directory exists and just create it if it doesn't.
I also plan on submitting some PR's with these windows related items once we have plan going forward so it's not all on you, I'm more than happy to help solve these issues on Windows.
Thought?
- no particular reason for that, but we can't just bet on all id's to be valid windows file names
- we can just call mkdir with pcall
- PR's would be welcome, although, I'm sorry to say that I don't know when I'll be able to merge that.. might be soon and it might take more than a month :(
- no particular reason for that, but we can't just bet on all id's to be valid windows file names
Right, which is why I was originally proposing we make it something else, if we use the string util that was added with my other PR that should work because it just a sequence of numbers and letters that are possible correct? Which should always be a viable directory name for Windows & Unix alike.
My current concern for using that entire file path is hitting the Windows character limit. Which is only 260 characters including c:\ and the null character at the end of the path.
Ex: On my computer at work that path total is currently sitting at 140 characters, but depending on how usernames are created for a user I would say it is very plausible that limit could get hit.
- we can just call mkdir with pcall
- PR's would be welcome, although, I'm sorry to say that I don't know when I'll be able to merge that.. might be soon and it might take more than a month :(
Calling with pcall
works as well, that didn't cross my mind, I update my fork and try pcall
to see if that works.
As for you schedule that's totally understandable. I would think most of us have fulltime jobs that are not creating Nvim/Vim plugins lol. So if my PR addresses the issues with Windows and still keeps Linux stable, I just stay on my forked branch until you are able to review again!
hey, I'm just now getting my feet wet with this plugin, and it seems like it's going to be awesome once it's working. I'm also on windows, and getting this issue.
I agree with @JustBarnt that bloating the length of the path is going to run into issues really quick on windows, and having the mix of path separators is really not great either, but the path thing is fixed quite nicely by making sure to call vim.fs.normalize on everywhere you plan on having paths constructed.
as far as the file name length goes, what if you just took the last generated bit from the id and stuck that as the dir name? like mine has this
"c:.local\state\nvim-data/dbee/notes/file_source_c:/.local/state/nvim-data/dbee/persistence.jsonLwGTlWTFbO/note_hwGawobXkc.sql"
if you ran a replace on it taking out "file_source_c:/.local/state/nvim-data/dbee/persistence.json" I'd end up with this: "c:.local\state\nvim-data/dbee/notes/LwGTlWTFbO/note_hwGawobXkc.sql", which would certainly help quite a bit, and then to run a normalize on it would result with "c:/.local/state/nvim-data/dbee/notes/LwGTlWTFbO/note_hwGawobXkc.sql" which is guaranteed not to flip out Windows.
also checking if a dir exists would make it easier, especially if you could just pass the final filepath wanted in, so, like in this case, if you added these funcs in:
local function dir_exists(path)
local stat = vim.loop.fs_stat(path)
local isDir = false
if stat then
vim.notify(vim.inspect(stat.type))
if stat.type == "file" then
vim.notify(vim.inspect(stat.type))
isDir = dir_exists(vim.fs.dirname(path))
end
if stat.type == "directory" then
isDir = true
end
end
return isDir
end
local function make_dir(path)
path = vim.fs.normalize(path)
local exists = dir_exists(path)
if not exists then
vim.uv.fs_mkdir(path, 1, function(err)
if err then
vim.notify("Error creating directory: " .. path .. " - " .. err)
else
vim.notify("Directory created: " .. path)
end
end)
else
vim.notify("Directory Already exists: " .. path)
end
end
you could pass in "c:\.local\state\nvim-data/dbee/notes/LwGTlWTFbO" and get a dir created with a normalized path.
could also get more "fancy" with it and take a dependency on plenary, which has a good path module, and you could take advantage of how it handles creating dirs, which I assume is pretty good.