lifepillar / vim-colortemplate

The Toolkit for Vim Color Scheme Designers!
929 stars 29 forks source link

fix for #74 #75

Closed christianrickert closed 1 year ago

christianrickert commented 1 year ago

Adding the :h modifier to the expand function resolves an issue with path truncation when the path contains a space character.

lifepillar commented 1 year ago

Mmh, the change you are proposing breaks almost all the tests, because :S causes the path to be enclosed in quotes, at least on Unix. For example, if:

echo expand('%')

is autoload/colortemplate.vim, then

echo fnamemodify(expand('%'), ':S')

is 'autoload/colortemplate.vim' (with single quotes).

Are you on Windows? I see two alternatives:

  1. escape only spaces (as it seems that the issue is about spaces);
  2. guard your proposal so that it is used only in the environments where it works (e.g., if has('win32') etc.).

While (2) would be simpler, I'm afraid that using :S might have other side effects, because the purpose of :S is to escape a path for using it in a shell command, but Colortemplate does not use paths in shell commands. So, I'd rather address the specific problem in a way that works everywhere (e.g., using :help escape()). This might require touching more parts of the code, though.

christianrickert commented 1 year ago

@lifepillar I've followed your first proposal to escape spaces in the file path and added the conditional back into the code.

Could you please test @43?

lifepillar commented 1 year ago

Thanks @christianrickert! The current proposal fixes the error when opening a template without breaking any test. There is still an issue, however, when building: I get Path is not a directory if the template's directory contains spaces.

I'm a bit overwhelmed currently, but hopefully I will be able to take a deeper look during the weekend.

lifepillar commented 1 year ago

Thanks for contributing! Even if this PR has not been merged, it has been instrumental in letting me understand the issue, and your feedback has been very valuable!