fix: correct path displayed in notification for generated files

[chee-zaram]

At the moment, when you specify a filepath using the output key, the generated message shows the current working directory before the actual filepath where the image is store.

Example:

silicon generated an image at "/home/cheezaram/dotfiles.pub/~/Pictures/2024-03-02T18-18-18_code.png"

or

silicon generated an image at "/home/cheezaram/dotfiles.pub/./Pictures/2024-03-02T18-18-18_code.png"

This PR changes that behaviour so you have

silicon generated an image at: ~/Pictures/2024-03-02T18-18-18_code.png

or

silicon generated an image at: ./Pictures/2024-03-02T18-18-18_code.png

[michaelrommel]

I like the idea a lot, but with your patch it doesn't handle the case with the silicon-config file anymore, where we do not know where the file was actually generated. If you could add that else branch to the PR, I would be happy!!

[michaelrommel]

I thought about it some more: I think leaving out the cwd when the output starts with ~ makes total sense. But in the other case, where it starts with "./" I'd rather then strip off those characters. Because people use vim-rooter etc, so the CWD may not be so obviously related to the file in the editor. And having the whole path makes it a lot easier to locate the image.

[chee-zaram]

I like the idea a lot, but with your patch it doesn't handle the case with the silicon-config file anymore, where we do not know where the file was actually generated. If you could add that else branch to the PR, I would be happy!!

I'm not quite sure I follow on this one. If you could go over your explanation again I'd be grateful.

[chee-zaram]

I thought about it some more: I think leaving out the cwd when the output starts with ~ makes total sense. But in the other case, where it starts with "./" I'd rather then strip off those characters. Because people use vim-rooter etc, so the CWD may not be so obviously related to the file in the editor. And having the whole path makes it a lot easier to locate the image.

I think I agree with you on this. I'll try to strip out the characters and prepend the CWD instead.

[chee-zaram]

I thought about it some more: I think leaving out the cwd when the output starts with ~ makes total sense. But in the other case, where it starts with "./" I'd rather then strip off those characters. Because people use vim-rooter etc, so the CWD may not be so obviously related to the file in the editor. And having the whole path makes it a lot easier to locate the image.

Done ✅

[michaelrommel]

Thank you very much! I'll merge the PR as is, but later I would have to slightly modify it for the case, where M.filenname == nil, which is the case when the user is using a silicon-config file.

[michaelrommel]

I like the idea a lot, but with your patch it doesn't handle the case with the silicon-config file anymore, where we do not know where the file was actually generated. If you could add that else branch to the PR, I would be happy!!

I'm not quite sure I follow on this one. If you could go over your explanation again I'd be grateful.

One of the users asked me specifically to deal with the case, where some options to the silicon command reside in silicon's own config file at ~/.config/silicon/config. So theoretically someone could have an output = "./code.png" directive there and that would then be missing in the lua table. So M.filename becomes nil and then the lua script throws an error. So the lines, that you changed dealt with that case, specifically

M.filename and x or y

means if M.filename is not nil take x otherwise take y.

[chee-zaram]

Okay. I understand you now. My bad. I'll let you handle it since you already merged.

[michaelrommel]

No problem, your improvements are still in there and I am grateful for those!! Already added the case back and tested all cases! Have a good weekend!

[chee-zaram]

I had actually fixed and pushed only to realize you beat me to it, haha. Thanks for merging. Your plugin is really helpful and nice. Enjoy your weekend, Michael.