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: normalize all frame filePath to posix format #46

Merged
merged 2 commits into from
Sep 2, 2022

Conversation

Julien-R44
Copy link
Member

@Julien-R44 Julien-R44 commented Sep 2, 2022

Okay so, I will explain why this modification is required for Windows users :

I wanted to add the feature of having a relative path rather than absolute path in the Japa error printer. But after investigating I noticed that this feature was already in place! Thanks to the displayShortPath property of youch-terminal.
But it doesn't work for Windows :

image

  • So the first thing we need to do is to normalize the Youch filePaths, so that they are all Posix. In our case, the filePaths that come out of StackTracey, are in Posix format, even for me who is under Windows.
    The only paths that are not in Posix are the ones coming out of fileURLToPath. The separator of the path is platform specific in this case. So that's what's I fixed in this PR

  • The second thing, which will be sent in a PR for youch-terminal, is to modify this line:
    https://github.com/poppinss/youch-terminal/blob/develop/index.js#L139
    We'll just convert the return of cwd(), which is platform specific, to posix format. And also don't use sep anymore but just a /.

Thanks to these modifications, I now have a nice relative path in Japa!

image

We could have done the opposite, and keep specific platform paths (and thus rather convert the paths of StackTracey). Personally, I prefer that we normalize all this. It makes the code more predictable, I think. If I miss something, let me know

Feel free to ask if it isn't clear ! 👍

@thetutlage
Copy link
Member

Thanks for the PR. A quick question.

Can you click on the path to open the file in your editor? In Mac I can do it. Is this even a thing in windows?

@Julien-R44
Copy link
Member Author

Yes and this feature also works with a posix path on Windows

@thetutlage
Copy link
Member

Wonderful

@thetutlage thetutlage merged commit cd66012 into poppinss:develop Sep 2, 2022
@thetutlage thetutlage self-requested a review September 2, 2022 23:02
@thetutlage
Copy link
Member

Released as 3.2.1

@Julien-R44 Julien-R44 deleted the fix/normalize-filepath branch September 3, 2022 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants