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 bug in drop_suffix #6321

Merged
merged 1 commit into from
Oct 26, 2022
Merged

Conversation

gridbugs
Copy link
Collaborator

Previously if the suffix was the entire string, drop_suffix would return the original string. This changes its behaviour to return the empty string instead in this case.

Signed-off-by: Stephen Sherratt [email protected]

Copy link
Collaborator

@Alizter Alizter left a comment

Choose a reason for hiding this comment

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

Do you know where we use this mostly?

@gridbugs
Copy link
Collaborator Author

Looks like it's mostly used to remove file extensions from filenames.

@rgrinberg
Copy link
Member

Could you add a unit test in stdune/string_tests.ml? I'm convinced the change is correct, but we should make sure there are no regressions as well.

@gridbugs gridbugs force-pushed the fix-String.drop_suffix branch from 0a97082 to ca51ba6 Compare October 26, 2022 01:26
Previously if the suffix was the entire string, drop_suffix would return
the original string. This changes its behaviour to return the empty
string instead in this case.

Signed-off-by: Stephen Sherratt <[email protected]>
@gridbugs gridbugs force-pushed the fix-String.drop_suffix branch from ca51ba6 to e8f3a88 Compare October 26, 2022 01:26
@gridbugs
Copy link
Collaborator Author

Could you add a unit test in stdune/string_tests.ml? I'm convinced the change is correct, but we should make sure there are no regressions as well.

Added!

@rgrinberg rgrinberg merged commit cf84807 into ocaml:main Oct 26, 2022
@Alizter Alizter mentioned this pull request Oct 27, 2022
jchavarri added a commit to jchavarri/dune that referenced this pull request Oct 27, 2022
* main:
  makefile: bind on 0.0.0.0 for livedoc (ocaml#6336)
  ci: rewrite fmt job (ocaml#6334)
  refactor(ci): cleanup names (ocaml#6335)
  fix: create fake socket rpc file on windows (ocaml#6329)
  refactor(doc): improvements to hacking.rst (ocaml#6324)
  ci: Add documentation job (ocaml#6333)
  Fix bug in drop_suffix (ocaml#6321)
  fix: public binaries with absolute build path
  test: public binaries in a cram test
  chore: update dune-project to 3.5 (ocaml#6328)
  chore(nix): add `devShells.slim` (ocaml#6327)
  test: remove obsolete bisect tests (ocaml#6318)
  refactor: clarify matching for path comparison and add a docstring (ocaml#6322)
@gridbugs gridbugs deleted the fix-String.drop_suffix branch October 11, 2023 02:30
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.

3 participants