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

Remove str2wcs special case for MB_CUR_MAX #9473

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

faho
Copy link
Member

@faho faho commented Jan 14, 2023

This meant we didn't actually do our weird en/decoding scheme for e.g. a C locale, which meant that, when you then switch to a proper locale the previous variables were broken.

I don't know how to test this automatically - none of my attempts seem to ever fail with the old code, here's what you'd do manually:

  • Run fish with an actual C locale (LC_ALL=C fish_allow_singlebyte_locale=1 fish)
  • set -gx foo 💩
  • set -e LC_ALL
  • echo $foo outputs "💩" if it works and "ð⏎" if it's broken.

Fixes #2613

TODOs:

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

This meant we didn't actually do our weird en/decoding scheme for e.g.
a C locale, which meant that, when you then switch to a proper locale
the previous variables were broken.

I don't know how to test this automatically - none of my attempts seem
to ever *fail* with the old code, here's what you'd do manually:

- Run fish with an actual C locale (LC_ALL=C
fish_allow_singlebyte_locale=1 fish)
- `set -gx foo 💩`
- `set -e LC_ALL`
- `echo $foo` outputs "💩" if it works and "ð⏎" if it's broken.

Fixes fish-shell#2613
@faho faho added the bug Something that's not working as intended label Jan 14, 2023
@faho faho requested a review from ridiculousfish January 14, 2023 08:15
Copy link
Member

@ridiculousfish ridiculousfish left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me. We should be moving towards using Unicode internally anyways.

@faho faho added this to the fish 3.6.1 milestone Jan 14, 2023
@faho faho merged commit 5c56fa0 into fish-shell:master Jan 14, 2023
@faho faho deleted the singlebyte-do-decode branch January 14, 2023 21:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variables saved with C locale are broken in other locales (was "iTerm2: Wrong $PWD after locale is changed")
2 participants