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 #12130 ; improve naming scheme in fakePackageName #12149

Merged
merged 2 commits into from
Sep 8, 2019

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Sep 6, 2019

../bam@bax/cab.nim
=>
..@bam@@bax@cab.nim.c

../bam/bax/cab.nim
=>
..@bam@bax@cab.nim.c

the leading .. could be avoided but IMO is more readable and should be legal anyway

note

this PR does not try to make the fakePackageName a valid nim identifier (shouldn't be needed IIUC); note that this also wasn't the case before this PR, eg:
nimcache/_7bar.baz7gaz77.nim.c doesn't give a valid identifier (for ../bar.baz/gaz7.nim); ditto with starting with _7 or containing __)

@timotheecour timotheecour changed the title fix #12130 ; improve naming scheme in fakePackageName [WIP] fix #12130 ; improve naming scheme in fakePackageName Sep 6, 2019
@Araq
Copy link
Member

Araq commented Sep 6, 2019

You need to adapt some tests I think and then it's ready to go. In retrospect, I would have produced subdirs inside nimcache, but this improves the situation, so thank you!

@timotheecour timotheecour changed the title [WIP] fix #12130 ; improve naming scheme in fakePackageName fix #12130 ; improve naming scheme in fakePackageName Sep 7, 2019
@timotheecour
Copy link
Member Author

You need to adapt some tests I think and then it's ready to go.

done

In retrospect, I would have produced subdirs inside nimcache, but this improves the situation, so thank you!

relative paths (..) is awkward to deal with subdirs though; and flat generated files seems easier to deal with (no need for rm -rf etc)

@dom96
Copy link
Contributor

dom96 commented Sep 7, 2019

Thank you for this!

@juancarlospaco
Copy link
Collaborator

its not supposed to do this ❔
https://nim-lang.github.io/Nim/os.html#quoteShell%2Cstring
Because if not would be a great addition to that proc, like an optional escapeSlash = true or something.
🤔

@timotheecour
Copy link
Member Author

@juancarlospaco not sure I understand how this relates to quoteShell; the goal of fakePackageName/demanglePackageName is to translate a relative path (possibly starting with ..) to a filename, and back. quoteShell has a different goal.

@Araq Araq merged commit f915064 into nim-lang:devel Sep 8, 2019
@timotheecour timotheecour deleted the pr_fix_12130 branch September 8, 2019 19:08
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.

Filenames ending with xxx8.nim
4 participants