Skip to content

feat: dont underscore numbers when formatting module names #655

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

Closed
wants to merge 58 commits into from

Conversation

c43721
Copy link

@c43721 c43721 commented Nov 7, 2024

Fixes jsr-io/jsr#803 but I'd like to know how or if I could better test this.

I eventually got here by looking at how the identifier for the file was created:
image

image

The identifier replacing all non-characters with underscores seems to be the source so adjusting the regex to allow numbers.

Though this regex might need to account for a leading number and replace that since you can't have a module identifier prefixed by a number (must be a character)

@CLAassistant
Copy link

CLAassistant commented Nov 7, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
7 out of 8 committers have signed the CLA.

✅ crowlKats
✅ dsherret
✅ c43721
✅ bartlomieju
✅ lucacasonato
✅ BlackAsLight
✅ kt3k
❌ denobot
You have signed the CLA already but the status is still pending? Let us recheck it.

@c43721
Copy link
Author

c43721 commented Nov 8, 2024

I'm not sure the regex would want to handle something as complex as "allow a-Za-z0-9 but if the string starts with 0-9 then prepend _" and I'm not sure that would be a valid export so I don't think that should be a concern here. Any inputs?

@c43721 c43721 force-pushed the dev/c43721/803-numbers-in-modules branch from 2a66a96 to 76fd786 Compare November 8, 2024 17:51
@crowlKats
Copy link
Member

@c43721 yes it should handle the case of a starting number

@c43721 c43721 force-pushed the dev/c43721/803-numbers-in-modules branch from 523e7ea to ada08ad Compare November 8, 2024 20:50
@crowlKats
Copy link
Member

@c43721 for test, you can update the module doc name in tests/testdata/multiple/b.ts

@c43721 c43721 force-pushed the dev/c43721/803-numbers-in-modules branch from ada08ad to 93dcd1c Compare November 21, 2024 19:06
@c43721 c43721 force-pushed the dev/c43721/803-numbers-in-modules branch from 3e0a7c4 to 7adc15c Compare April 11, 2025 14:06
@c43721
Copy link
Author

c43721 commented Apr 11, 2025

I'm just going to re-do this under a new branch.

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.

Import string replaces numbers with underscores
9 participants