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

[Bug] second .jl is added when package name is entered with .jl attached #373

Closed
jhidding opened this issue Jul 23, 2024 · 7 comments · Fixed by #377
Closed

[Bug] second .jl is added when package name is entered with .jl attached #373

jhidding opened this issue Jul 23, 2024 · 7 comments · Fixed by #377
Assignees
Labels
bug Something isn't working

Comments

@jhidding
Copy link

Description

When asked for the package name, a not too careful reader may enter a package name with .jl included, resulting in a double MyPackage.jl.jl. This could be easily caught.

Package Version

v0.9.0

Julia Version

1.10

Reproduction steps

run BestieTemplate.generate enter a package name with .jl

Relevant log output

Your package ModuleMixins.jl.jl has been created successfully! 🎉

Operating System

Linux

@jhidding jhidding added the bug Something isn't working label Jul 23, 2024
@abelsiqueira abelsiqueira self-assigned this Jul 23, 2024
@abelsiqueira
Copy link
Owner

I could not reproduce, did you run BestieTemplate.generate("ModuleMixins.jl")?

In theory this shouldn't happen via the Julia interface because we run _sanitize_package_name on the path unless PackageName is manually set. You should see

[ Info: Using sanitized path ModuleMixins as package name

in the first line of log.

@jhidding
Copy link
Author

No, I had the path already there, so I ran BestieTemplate.generate(".")

@abelsiqueira
Copy link
Owner

I can reproduce it, although it's not a bug.

The . is processed correctly as a lack of name, so the it moves on to the Copier side without a question. Then you get asked by Copier for the name, so it's out of ours hands.

There are a few ways to deal with this, though:

  1. Assume that . means that the package name is the name of the folder. I don't like it because it's not my business what happens in ...
  2. Assume that . is a mistake, and disallow it. I don't like it because it might be the case that people know what they're doing.
  3. Enforce rules on PackageName to disallow . in the name.

If you agree with the assessment, I will look into 3.

abelsiqueira added a commit that referenced this issue Jul 24, 2024
Uses regex to make sure that the package name starts
with a capital letter and contains only letters and numbers.

Closes #373
@abelsiqueira abelsiqueira mentioned this issue Jul 24, 2024
5 tasks
@jhidding
Copy link
Author

Though a bit pedantic, I agree with the assessment. I think Copier lets you (at least) validate (https://copier.readthedocs.io/en/stable/configuring/#advanced-prompt-formatting) answers. Disallowing . in the name is the more fundamental approach.

@abelsiqueira
Copy link
Owner

Thanks, it's done in #377 now.

@abelsiqueira
Copy link
Owner

@allcontributors please add @jhidding for bug

Copy link
Contributor

@abelsiqueira

I've put up a pull request to add @jhidding! 🎉

abelsiqueira added a commit that referenced this issue Jul 24, 2024
Uses regex to make sure that the package name starts
with a capital letter and contains only letters and numbers.

Closes #373
abelsiqueira added a commit that referenced this issue Jul 24, 2024
Uses regex to make sure that the package name starts
with a capital letter and contains only letters and numbers.

Closes #373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants