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

Do not blindly lowercase language names #37

Merged
merged 4 commits into from
Dec 4, 2019
Merged

Do not blindly lowercase language names #37

merged 4 commits into from
Dec 4, 2019

Conversation

jspitz
Copy link
Contributor

@jspitz jspitz commented Dec 4, 2019

Rather than that, only do that if they are completely uppercased
(which is probably the cause of a \MakeUppercase e.g. in headings)

This assures mixed-case names such as USenglish are treated correctly

Fixes issue #12.

This revokes #13 which was screwed.

I've addressed #13 (comment)

If you dislike the approach in general, feel free to close this.

jspitz added 3 commits April 24, 2017 13:31
Rather than that, only do that if they are completely uppercased
(which is probably the cause of a \MakeUppercase e.g. in headings)

This assures miced-case names such as USenglish are treated correctly

Fixes issue #12.
@jspitz
Copy link
Contributor Author

jspitz commented Dec 4, 2019

The \ifx test here is wrong (comparing the first two tokens of #1 or perhaps a single #1 token to {, etc.

This has been addressed.

csquotes.sty Outdated Show resolved Hide resolved
csquotes.sty Show resolved Hide resolved
@josephwright
Copy link
Owner

Looks broadly good, though I'd like some groups/private vars. Longer-term, with the expl3 pre-loading plan, I might look to use case folding here. (I should also talk to Javier able babel in this regard.)

@jspitz
Copy link
Contributor Author

jspitz commented Dec 4, 2019

All comments have been addressed.

@josephwright josephwright self-requested a review December 4, 2019 09:16
@josephwright josephwright merged commit 6dbffe1 into josephwright:master Dec 4, 2019
\begingroup
\def\csq@tempa{#1}%
\def\csq@tempb{\MakeUppercase{#1}}%
\ifx\csq@tempa\csq@tempb

Choose a reason for hiding this comment

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

isn't this test always false, given the two non equal definitions above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I forgot \MakeUppercase is not expandable.

@josephwright please revert my commit.

Choose a reason for hiding this comment

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

ifx would be false even if it were expandable (but you could have fixed it with edef in that case)

Copy link

@davidcarlisle davidcarlisle Dec 4, 2019

Choose a reason for hiding this comment

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

assuming that the names are always ascii (so the later \lowercase is safe) you could do \uppercase{\def\csq@tempb{#1}} then \ifx\csq@tempa\csq@tempb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this seems to work.

diff --git a/csquotes.sty b/csquotes.sty
index dacb622..50a5401 100644
--- a/csquotes.sty
+++ b/csquotes.sty
@@ -832,7 +832,7 @@
 \def\csq@lc@if@uc#1{%
  \begingroup
  \def\csq@tempa{#1}%
- \def\csq@tempb{\MakeUppercase{#1}}%
+  \uppercase{\def\csq@tempb{#1}}%
   \ifx\csq@tempa\csq@tempb
      \lowercase{#1}%
   \else

@josephwright, interested?

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