Skip to content

Normalize slashes in paths in TASTy binaries #10678

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

Merged

Conversation

abgruszecki
Copy link
Contributor

@abgruszecki abgruszecki commented Dec 7, 2020

[test_windows_full]

@abgruszecki
Copy link
Contributor Author

@liufengyun Asking since you were involved with similar PRs, also involving Windows. It seems like cmdTests are not run on Windows. This PR ensures that we always use / to separate path elements in TASTy binary files, can you think of any way of testing that on Windows?

@abgruszecki
Copy link
Contributor Author

@bishabosha since you worked around TASTy reader, are there any tests for it that get ran on Windows?

@bishabosha
Copy link
Member

@abgruszecki One way i normalised paths was in ExtractSemanticDB where the path is converted to javas URI

@abgruszecki abgruszecki added this to the 3.0.0-M3 milestone Dec 7, 2020
@abgruszecki abgruszecki changed the title Normalize tasty path slash Normalize slashes in paths in TASTy binaries Dec 7, 2020
@abgruszecki
Copy link
Contributor Author

@nicolasstucki Should we mention somewhere that the paths in tasty binaries always use forward slashes?

@nicolasstucki
Copy link
Contributor

nicolasstucki commented Dec 7, 2020

Documentation for this is a good idea.

  • Can we add it to the TastyFromat?
  • We should also add it in the reflection API
  • It would be good to repeat it in the method that normalizes the path to make sure the next person that modifies it knows about this.

@liufengyun
Copy link
Contributor

@liufengyun Asking since you were involved with similar PRs, also involving Windows. It seems like cmdTests are not run on Windows. This PR ensures that we always use / to separate path elements in TASTy binary files, can you think of any way of testing that on Windows?

Yes, cmdTests only run on Linux machines. Currently, we don't have scripts that run on Windows. A possibility would be to create a macro test, and print the relative path of a symbol.

@abgruszecki
Copy link
Contributor Author

@nicolasstucki where should we modify TastyFormat?

@abgruszecki
Copy link
Contributor Author

And yes, I'll also modify the Quotes API.

@abgruszecki abgruszecki force-pushed the normalize-tasty-path-slash branch 3 times, most recently from 974afe5 to b6f38c8 Compare December 8, 2020 16:38
@abgruszecki abgruszecki force-pushed the normalize-tasty-path-slash branch from b30887e to 1bb5fd4 Compare December 9, 2020 11:35
@abgruszecki abgruszecki force-pushed the normalize-tasty-path-slash branch from 1bb5fd4 to 0c17ff4 Compare December 9, 2020 12:20
@abgruszecki abgruszecki marked this pull request as ready for review December 9, 2020 15:26
@nicolasstucki nicolasstucki merged commit 782adbb into scala:master Dec 9, 2020
@nicolasstucki nicolasstucki deleted the normalize-tasty-path-slash branch December 9, 2020 15:26
@Kordyjan Kordyjan modified the milestones: 3.0.0-M3, 3.0.0 Aug 2, 2023
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.

5 participants