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

feat: dont underscore numbers when formatting module names #655

Open
wants to merge 5 commits into
base: main
Choose a base branch
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
All committers have signed the CLA.

@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
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
3 participants