search

dbrgn / tealdeer

A very fast implementation of tldr in Rust.
https://dbrgn.github.io/tealdeer/
Apache License 2.0
4.09k stars 124 forks source link

Use XDG conventions on macOS too #319

Open utkarshgupta137 opened 1 year ago

utkarshgupta137 commented 1 year ago

Fixes #311.

dbrgn commented 1 year ago

Hi, thanks for the PR. Some questions before I take a closer look:

utkarshgupta137 commented 1 year ago

Hi, thanks for the PR. Some questions before I take a closer look:

* In the base configuration (if no XDG variables are set), will this use the same directories as before on non-macOS systems? Have you verified that this does not modify the directory paths on Windows and Linux?

* In [#311](https://github.com/dbrgn/tealdeer/issues/311), the request was to use XDG conventions _if_ XDG variables are set, but not by default. This makes sense, as we should follow the local OS conventions by default. Does etcetera implement this logic? From what I can tell, it will always use XDG conventions. This would be a breaking change for Tealdeer users on macOS (not just the cache will be created a second time in a different location without the old cache being cleaned up, but if a config file is present in the previous location, it will be ignored).

Ugh, looks like app_dirs2 doesn't follow the convention on Windows of most other "dirs" crates out there. Linux users won't be affected, but I can modify the code to retain the same behavior on Windows. I don't use CLI tools on Windows, so I've no strong opinions about it.

No, right now this PR makes no effort to maintain backward compatibility on macOS. IMO, no macOS users expect the config file to be located in ~/Library/Application Support. Almost every CLI tool which uses the dirs/dirs-next crates & other clones has open or closed issues of users wanting to use the XDG conventions on macOS. But I've yet to come across a case where the inverse is true. I've written more about this here. Secondly, I don't think it is a good idea to half-ass the XDG convention. If a user who has previously not set it decides to set it for whatever reason, then they may be confused why some unrelated tool is broken. Same if they remove it for any reason. However, since you probably want to maintain backward compatibility, I can add migration code to move the old data to the new location, similar to this: https://github.com/cantino/mcfly/pull/349/files#diff-4f91d834bba4a3a432715f3ccf4ebc7c10d7ecfb974f1e78f56108dd80581a53R392-R404

utkarshgupta137 commented 1 year ago

Fixed & test on Windows:

PS C:\Users\Utkarsh Gupta\Downloads\tealdeer-main> ..\tealdeer-windows-x86_64-msvc.exe --show-paths
Config dir:       C:\Users\Utkarsh Gupta\AppData\Roaming\tealdeer\tealdeer\ (OS convention)
Config path:      C:\Users\Utkarsh Gupta\AppData\Roaming\tealdeer\tealdeer\config.toml
Cache dir:        C:\Users\Utkarsh Gupta\AppData\Local\tealdeer\tealdeer (OS convention)
Pages dir:        C:\Users\Utkarsh Gupta\AppData\Local\tealdeer\tealdeer\tldr-pages\
Custom pages dir: C:\Users\Utkarsh Gupta\AppData\Local\tealdeer\tealdeer\pages\ (OS convention)

PS C:\Users\Utkarsh Gupta\Downloads\tealdeer-main> .\target\release\tldr.exe --show-paths
Config dir:       C:\Users\Utkarsh Gupta\AppData\Roaming\tealdeer\tealdeer\ (Windows convention)
Config path:      C:\Users\Utkarsh Gupta\AppData\Roaming\tealdeer\tealdeer\config.toml
Cache dir:        C:\Users\Utkarsh Gupta\AppData\Local\tealdeer\tealdeer (Windows convention)
Pages dir:        C:\Users\Utkarsh Gupta\AppData\Local\tealdeer\tealdeer\tldr-pages\
Custom pages dir: C:\Users\Utkarsh Gupta\AppData\Local\tealdeer\tealdeer\pages (Windows convention)
utkarshgupta137 commented 1 year ago

I should mention that is weird that app_dirs2 uses the same location for cache & user data. The popular directories and most of its clones put the data & config together, not the data & cache. Edit: According to many articles, the Local folder is not supposed to be synced while the Roaming folder is supposed to be synced. So I guess it would make sense to have the custom pages dir in the Roaming folder. Again, I don't use CLI on Windows, so I don't have a strong opinion.

utkarshgupta137 commented 1 year ago

@dbrgn If you want, I can add a fallback to ~/Library/Application Support as well.

utkarshgupta137 commented 1 year ago

@dbrgn ping

niklasmohrin commented 1 year ago

Let's discuss the transition period in the issue thread first :)

dbrgn commented 2 months ago

@utkarshgupta137 thanks for your work so far. Do you plan to carry on with this PR? If yes, it would need to be updated as per https://github.com/dbrgn/tealdeer/issues/311#issuecomment-1937787422

utkarshgupta137 commented 2 months ago

Sorry, I don't use this tool very much & generally don't have enough bandwidth to continue. But I would be happy to review PRs or assist otherwise.