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 definition of \IfLabelExistsT, \IfLabelExistsF #1476

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

mbertucci47
Copy link
Contributor

@mbertucci47 mbertucci47 commented Sep 25, 2024

READ ME FIRST: Please understand that in most cases we will not be able to merge a pull request because there are a lot of internal activities needed when updating the LaTeX2e sources. If you have a code suggestion please discuss it with the team first.

Pull requests in this repository are intended for LaTeX Team members only.

Internal housekeeping

The definitions for \IfLabelExistsT and \IfLabelExistsF are currently the same as \IfPropertyExistsT and \IfPropertyExistsF which is wrong. They should be defined with \property_if_recorded:... like \IfLabelExistsTF.

I changed the date but not the version and did not add a changed entry. Let me know if either is necessary.

Status of pull request

  • Ready to merge

Checklist of required changes before merge will be approved

  • Test file(s) added
  • Version and date string updated in changed source files
  • Relevant \changes entries in source included
  • Relevant changes.txt updated
  • Rollback provided (if necessary)?
  • ltnewsX.tex (and/or latexchanges.tex) updated

@mbertucci47
Copy link
Contributor Author

I see it's failing a test. I don't know enough about l3build to know which command to run to regenerate the files, please let me know and I'll do that ASAP

@u-fischer
Copy link
Member

u-fischer commented Sep 25, 2024

Ups. Yes I think an \changes entry, a version change and also an entry in changes.txt is needed

The test is failing because it is contains the same copy&paste error:

\IfLabelExistsT {name\propsuffix}{true}

that should be \labelsuffix.

@mbertucci47
Copy link
Contributor Author

mbertucci47 commented Sep 25, 2024

I've added a changes entry, version update, entry to changes.txt, and fixed the test file. Hopefully should be good to go now

@u-fischer u-fischer self-requested a review September 25, 2024 20:31
Copy link
Member

@FrankMittelbach FrankMittelbach left a comment

Choose a reason for hiding this comment

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

looks good to me (assuming the tests now pass)

@josephwright josephwright merged commit f008dc9 into latex3:develop Sep 26, 2024
42 checks passed
@muzimuzhi
Copy link
Contributor

ltnewsX entry needed?

@FrankMittelbach
Copy link
Member

I think not really, it is a straight forward bug in stuff that was made available only recently. But of course it wouldn't hurt either.

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.

5 participants