Skip to content

Handle non-displayable characters when detecting an erroneous package name or version#6640

Merged
rjbou merged 4 commits intoocaml:masterfrom
kit-ty-kate:invalid-char-utf8
Aug 25, 2025
Merged

Handle non-displayable characters when detecting an erroneous package name or version#6640
rjbou merged 4 commits intoocaml:masterfrom
kit-ty-kate:invalid-char-utf8

Conversation

@kit-ty-kate
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate commented Aug 13, 2025

Follow up to #6638
Queued on #6638
related to #6396

@kit-ty-kate kit-ty-kate added this to the 2.5.0~alpha1 milestone Aug 13, 2025
@kit-ty-kate kit-ty-kate added AREA: UI PR: QUEUED Pending pull request, waiting for other work to be merged or closed labels Aug 13, 2025
@kit-ty-kate kit-ty-kate removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Aug 13, 2025
Copy link
Copy Markdown
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the idea, lgtm! Some comment though

  • add the test (opam show bépo) before the fix
  • changelog
  • a more explicit comit message for commit #2: minor simplification in OpamFormula.atom_of_string for ex

# Return code 2 #
### opam show bépo
opam: PACKAGES… arguments: Invalid character '' in package name
opam: PACKAGES… arguments: Invalid character '\195' in package name
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these test be on their specific file ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with just error cases or something like that? Why not, although i don't feel a strong need for it personally

@kit-ty-kate
Copy link
Copy Markdown
Member Author

Some comment though

yeah sorry, i didn't clean up the PR at the time because it was queued on another PR so it needed rebasing anyway and i forgot to do it during the post-merge rebase.

@rjbou rjbou merged commit 9e638fd into ocaml:master Aug 25, 2025
44 checks passed
@kit-ty-kate kit-ty-kate deleted the invalid-char-utf8 branch August 25, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants