Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace dirs-next crate with etcetera crate #1304

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

utkarshgupta137
Copy link
Contributor

The config path is unchanged. The etcetera crate allows you to use the XDG spec on macOS too without any hassle. In addition, it is slightly smaller.

Some(err) => {
print_error(format!("Malformed pattern in global ignore file. {}.", err));
if let Ok(basedirs) = etcetera::choose_base_strategy() {
let global_ignore_file = basedirs.config_dir().join("fd").join("ignore");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to do something like:

if let Ok(appdirs) = etcetera::choose_app_strategy(etcetera::AppStrategyArgs {
    top_level_domain: "io.github".to_string(),
    author: "sharkdp".to_string(),
    app_name: "fd".to_string(),
}) {
    let global_ignore_file = appdirs.config_dir().join("ignore");

although, I think that would change the location on windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would.

@tmccombs
Copy link
Collaborator

I think the behaviro on windows is a little different. etcetera just joins "AppData\Roaming" to the users home directory, but dirs_next calls shlobj::SHGetKnownFolderPath. I'm not entirely sure how things work in windows, but https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid says that that is the "Default Path" for the RoamingAppData folder. which makes me think it is possible it could be something else.

@utkarshgupta137
Copy link
Contributor Author

I think the behaviro on windows is a little different. etcetera just joins "AppData\Roaming" to the users home directory, but dirs_next calls shlobj::SHGetKnownFolderPath. I'm not entirely sure how things work in windows, but https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid says that that is the "Default Path" for the RoamingAppData folder. which makes me think it is possible it could be something else.

I've created a PR for etcetera. I'll wait for a review, then merge it & release a new version.

@utkarshgupta137
Copy link
Contributor Author

I think the behaviro on windows is a little different. etcetera just joins "AppData\Roaming" to the users home directory, but dirs_next calls shlobj::SHGetKnownFolderPath. I'm not entirely sure how things work in windows, but https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid says that that is the "Default Path" for the RoamingAppData folder. which makes me think it is possible it could be something else.

This is addressed in etcetera v0.8

@sharkdp sharkdp merged commit 3ae0454 into sharkdp:master Jun 14, 2023
@sharkdp
Copy link
Owner

sharkdp commented Jun 14, 2023

Thank you. If I understand correctly, there is no change for our users, so we don't really need a CHANGELOG entry for this? If this is not the case, it would be great if you could open a quick PR with a CHANGELOG entry.

@utkarshgupta137
Copy link
Contributor Author

Thank you. If I understand correctly, there is no change for our users, so we don't really need a CHANGELOG entry for this? If this is not the case, it would be great if you could open a quick PR with a CHANGELOG entry.

No, no user-facing changes are expected. Same for the PRs for bat & vivid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants