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

Allow underscores as separators in the math builtin #8611

Merged
merged 6 commits into from
Mar 13, 2022
Merged

Allow underscores as separators in the math builtin #8611

merged 6 commits into from
Mar 13, 2022

Conversation

andmis
Copy link
Contributor

@andmis andmis commented Jan 4, 2022

Description

This change allows underscores as visual separators in the math builtin, so math 1_000 + 2_000 will return 3000. We allow leading, trailing, and multiple underscores. See the documentation of the fish_wcstod_underscores function in the PR.

See also #8607.

Fixes issue #8496.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

src/fish_tests.cpp Show resolved Hide resolved
doc_src/cmds/math.rst Outdated Show resolved Hide resolved
src/wutil.cpp Outdated Show resolved Hide resolved
src/wutil.cpp Outdated Show resolved Hide resolved
src/wutil.cpp Outdated Show resolved Hide resolved
src/wutil.cpp Outdated Show resolved Hide resolved
src/wutil.cpp Outdated Show resolved Hide resolved
@krobelus
Copy link
Member

krobelus commented Jan 9, 2022

looks really nice, I only have nitpicks

@andmis
Copy link
Contributor Author

andmis commented Jan 9, 2022

Thanks for the review. A missed null pointer check isn't a nitpick. :)

I've made the changes you requested -- one thing is that I added a changelog entry and I'm not sure about where I put it, so before merging someone should double-check that.

@zanchey
Copy link
Member

zanchey commented Jan 10, 2022

I want to hold this until after 3.4.0 is out as we're doing the final QA for the release now.

@andmis
Copy link
Contributor Author

andmis commented Jan 10, 2022

Yep that makes sense -- we'll just have to move the changelog entry before merging (it's under 3.4 right now).

@faho faho merged commit 59e50f7 into fish-shell:master Mar 13, 2022
@faho faho modified the milestones: fish-future, fish 3.5.0 Mar 13, 2022
@faho
Copy link
Member

faho commented Mar 13, 2022

Merged now, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants