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

Fix multiple paths: define the platform-specific join separator #840

Merged
merged 10 commits into from Dec 10, 2023

Conversation

zjp-CN
Copy link
Contributor

@zjp-CN zjp-CN commented May 13, 2023

fix #576

related to denisidoro#576

For the following config

cheats:
  paths:
    - C:\\Users\\Administrator\\AppData\\Roaming\\navi\\cheat
    - C:\\Users\\Administrator\\AppData\\Roaming\\navi\\cheat

navi now gets incorrect paths on Windows, since the seperator `:` for
path join is a valid component.

[2023-05-12T08:58:26Z DEBUG navi::commands::core] Filesystem(
        Some(
            "C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat:C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat",
        ),
    )
[2023-05-12T08:58:28Z DEBUG navi::filesystem] filesystem::Fetcher = Fetcher {
        path: Some(
            "C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat:C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat",
        ),
        files: RefCell {
            value: [],
        },
    }
2023-05-12T15:51:54.280707Z DEBUG navi::filesystem: read cheat files in `"C"`: []
2023-05-12T15:51:54.281083Z DEBUG navi::filesystem: read cheat files in `"\\Users\\Administrator\\AppData\\Roaming\\navi\\cheats"`: []
@zjp-CN zjp-CN requested a review from denisidoro as a code owner May 13, 2023 03:35
@welcome
Copy link

welcome bot commented May 13, 2023

Thanks for opening this pull request!

@zjp-CN
Copy link
Contributor Author

zjp-CN commented May 13, 2023

But the preview is broken too in debug mode.
20230513220046

The path is not right.
20230513213140

I think it's related to rust-lang/rust#99931 and rust-lang/rust#59117

@denisidoro
Copy link
Owner

@zjp-CN sorry for taking so long to review this. I'm not able to test this on a Windows machine. Is this a change you're comfortable with landing?

@jackandroselv
Copy link

jackandroselv commented Sep 24, 2023 via email

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Sep 25, 2023

Is this a change you're comfortable with landing?

Yes, I use the compiled navi based on this PR on Windows all the time, and it works fine.

This PR mainly fixes the path parsing on Windows (so nothing changes on Linux), and introduces tracing for logging.

@denisidoro denisidoro merged commit b560ba5 into denisidoro:master Dec 10, 2023
1 check passed
Copy link

welcome bot commented Dec 10, 2023

Congrats on merging your first pull request!

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.

Nothing of cheats available on Windows
3 participants