vim-suda
vim-suda copied to clipboard
suda#read throws uncaught exception when opening suda:///not-existing.txt
Hey,
I noticed that if I launch nvim suda:///not-existing.txt and /not-existing.txt doesn't exist, I get an exception:
Error detected while processing function suda#BufReadCmd[5]..suda#read:
line 28:
E605: Exception not caught:
Here's where this happens, for reference:
https://github.com/lambdalisue/suda.vim/blob/b867074b30f623ccb81a0bb8d1b74a6d0fafd160/autoload/suda.vim#L70-L77
cat fails because the file doesn't exist. Technically this error is harmless because if the file doesn't exist the buffer should be empty anyway. Perhaps this 'bug' shouldn't even be fixed?
We could ignore any errors but that won't be reliable if e.g the password was incorrect. As I'm thinking about it, this part of suda should be implemented with extreme care because if no error is thrown, a user might think he opened an empty file, add something according to some manual, save it (without errors) and end up with a totally different file with all the previous content lost.
According to my tests, even a wrong password ends up with an empty buffer and the same exception error is printed. Maybe suda should clearly state to the user what command failed and that the buffer's contents don't correspond to the real contents? Unless of course the file doesn't exist.. Anyway, I wonder what is your opinion on the subject.
Thanks.
It sounds really complicated 🤕
Well in the usual case, I think we can use getftype() to check if the file is not readable or does not exist (not 100% sure). The function would return values even the user does not have permission to read the file.
What I'm not sure is the situation when a user does not have permission to read the parent directory. I'm not sure if getftype() returns value in that case.
Anyway, I agree to NOT silence the error. We need deep investigation before applying fixes.
What I'm not sure is the situation when a user does not have permission to read the parent directory. I'm not sure if getftype() returns value in that case.
It returns an empty string in this case.
I've been thinking about the UX of the 'wrong password' vs 'other legitimate reasons for the buffer to appear empty' and I've had an idea:
When a suda:// buffer is opened, we should first check if the parent directory is readable and then check if we can be sure the file exists. In case we know it's there, and that cat command failed, the user should be prompted to try to read the command again and again until cat doesn't fail. Only if he wishes to quit this loop he'll be represented with a warning saying that the buffer's contents probably don't fit the real contents of the file.
In case the parent directory of the requested file doesn't exist, suda should still suggest to read the file with sudo cat again and again in the same manner as above (with a "would you like to try read it again y/n" prompt) until there's no v:shell_error. However, in this case, the warning message should be a little bit more calmed since we can't be sure of that there is such file or not.
Perhaps this kind of behavior is overwhelming but I think it'll be most liable.
I think there are following two distinct issues
- suda.vim should repeat the command until a correct password is given like normal
sudocommand onsuda#system - suda.vim should NOT read a content from non existing file on
suda#BufReadCmdbecause nowsuda://xxxxxis used for non existing files as well
So I'd love to
- Fix 1st issue
- Add a way to find if a file exists or not (maybe by
sudo lsor whatever) - Use above and skip
suda#readonsuda#BufReadCmdwhen the target does not exist
Oh right, I guess it'll be better to fix this in steps but there are a few details I think that should be discussed as for the implementation, per issue:
- Perhaps this repetition could be implemented in a
:term? Certainly this may be too verbose but I think it has the advantage that the user can see the error message clearly and exactly what command produced it, all in the:term. In case thissudo catcommand's exit is false,the user could decide according to the stderr of it whether it ought to try and run that command again. - Using
sudo lssounds good, In addition to the above suggestion, perhaps it will be better to use[[ -f <file> ]]instead?