nano-win icon indicating copy to clipboard operation
nano-win copied to clipboard

utils: ensure to return a slash-style homedir

Open chawyehsu opened this issue 3 years ago • 11 comments

This fixes the issue of including nanorc files whose path leads with a tilde.

before:

~/.nanorc

include "~/.nano/nanorc.nanorc"

~/.nano/nanorc.nanorc was expanded to C:\Users\<username>/.nano/nanorc.nanorc, and this doesn't work.

after:

~/.nano/nanorc.nanorc is expanded to C:/Users/<username>/.nano/nanorc.nanorc , work as expected.

chawyehsu avatar Dec 08 '20 03:12 chawyehsu

It's better to have this conversion in get_full_path(), instead of just applying it to the path to home directory. And I have it there now: https://github.com/lhmouse/nano-win/commit/ef33c2a8d638910861d2563d1ea75f5bf4407177#diff-57b320cc7adea8a7d492648179ebdd206bb8090f0b897e8965b0b482d0dbc88bR1247

lhmouse avatar Dec 08 '20 09:12 lhmouse

That looks better. Thank you for the good work!

chawyehsu avatar Dec 08 '20 09:12 chawyehsu

@lhmouse I've updated to the latest version of nano-win. It seems that your patches do not solve the issue I mentioned above. I'm reopening this, we need further discussion maybe.

Looks like the function calling stack in rcfile.c did not call get_full_path():

parse_rcfile() -> parse_includes() -> parse_one_include() -> real_dir_from_tilde() -> get_homedir()

Patching get_full_path() can not fix rcfiles including issue.

chawyehsu avatar Dec 08 '20 13:12 chawyehsu

Yeah we have to do the same to the path returned by real_dir_from_tilde(). I have a quick look at other functions that return char*s: It looks like that these two functions (as well as get_full_path()) may generate paths with slashes, while all the others just forward the result of them, so translating backslashes to forwardslashes in all results from them should suffice.

https://files.lhmouse.com/nano-win/nano-win_9371_v5.4-9-g8ad13081.7z

lhmouse avatar Dec 08 '20 14:12 lhmouse

Maybe we can just use get_full_path() inside the parse_one_include(), instead of duplicating slashes replacement in real_dir_from_tilde().

I just committed the patch and found you've rebased the branch...

chawyehsu avatar Dec 08 '20 14:12 chawyehsu

Really confused about branch rebasing, god.

chawyehsu avatar Dec 08 '20 14:12 chawyehsu

get_full_path() is actually calling real_dir_from_tilde() everytime, therefore doing slashes replacement in real_dir_from_tilde() only should be the minimal patch for all cases.

Edit: the function call

chawyehsu avatar Dec 08 '20 14:12 chawyehsu

This whack-a-mole game isn't satisfactory, given the fact that paths in principle are very fragile (some of them come from environment variables such as HOME, some others are hardcoded such as /tmp, and there is no proper check for path separators other than / at all).

Really confused about branch rebasing, god.

A preferred solution would be a standalone patch, such as https://github.com/lhmouse/nano-win/blob/master/ncurses-6.1.patch. But as GNU nano doesn't have a relatively stable working tree (the author really loves trivial changes such as renaming or deleting variables), this would result in a lot of conflicts frequently, and resolving git am'd conflicts is really a nightmare. So, rebase only, then force-push. Historical snapshots can be pushed to other branches.

lhmouse avatar Dec 08 '20 14:12 lhmouse

Looks like recent builds of nano-win breaked the slash fix, and did not handle ~/ includes anymore. version: v5.6.1-51-g32071383

chawyehsu avatar May 24 '21 15:05 chawyehsu

I should look into it later.

lhmouse avatar May 25 '21 01:05 lhmouse

closing stale pr.

chawyehsu avatar Feb 20 '24 04:02 chawyehsu