lhmouse / nano-win

GNU nano text editor for Windows [WARNING: The master branch is constantly rebased and force-pushed so don't expect it to be steady!! -end WARNING]
https://files.lhmouse.com/nano-win/
GNU General Public License v3.0
210 stars 25 forks source link

utils: ensure to return a slash-style homedir #20

Closed chawyehsu closed 6 months ago

chawyehsu commented 3 years ago

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.

lhmouse commented 3 years ago

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

chawyehsu commented 3 years ago

That looks better. Thank you for the good work!

chawyehsu commented 3 years ago

@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.

lhmouse commented 3 years ago

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

chawyehsu commented 3 years ago

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 commented 3 years ago

Really confused about branch rebasing, god.

chawyehsu commented 3 years ago

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

lhmouse commented 3 years ago

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.

chawyehsu commented 3 years ago

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

lhmouse commented 3 years ago

I should look into it later.

lhmouse commented 3 years ago

A new build has been uploaded at https://files.lhmouse.com/nano-win/nano-win_9605_v5.7-34-g387900675.7z.

chawyehsu commented 6 months ago

closing stale pr.