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.

Sign up for GitHub

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

Jump to bottom

\diffref and \diffrefs now take a comma-separated list of references. #2239

Merged
merged 3 commits into from Jan 18, 2019
Merged

\diffref and \diffrefs now take a comma-separated list of references. #2239

merged 3 commits into from Jan 18, 2019

Conversation

godbyk
Copy link
Contributor

@godbyk godbyk commented Jul 2, 2018

An improvement to #2236: The \diffref and \diffrefs macros now take a single argument, a comma-separated list of references.

The only difference in output is that now the word and is inserted prior to the penultimate reference if there are more than one. For example:

\diffref{sequence.reqmts,associative.reqmts}

Affected subclauses: 27.2.3 and 27.2.6

\diffrefs{complex.numbers}

Affected subclause: 30.5

\diffref{istream::sentry, ostream::sentry, iostate.flags}

Affected subclauses: 31.7.4.1.3, 31.7.5.1.3, and 31.5.5.4

Notes

  • I can remove the word and if you prefer and just emit a comma-separated list of references.
  • The \diffref and \diffrefs macros are exactly equivalent and may be used interchangeably. We can also remove one if you prefer.
  • Spaces around references are ignored so the comma-separated list may use spaces before and/or after the comma to improve readability.
  • subclause is automatically pluralized if more than one reference is provided.
  • I used the expl3 (LaTeX 3) macros for the comma-separated list processing. While there are other packages we could use, this one provides better functionality and more facilities for improving other macros in the future. I would love to be able to take advantage of expl3, but if you prefer to stick to LaTeX 2-based packages, I can rewrite the code to use one of them instead. (Note that using expl3 doesn't require using a new pdflatex binary. It's just a set of newer macros built atop the existing engines.)

@zygoloid
Copy link
Member

zygoloid commented Jul 2, 2018

How does one determine if they have LaTeX 3?

@godbyk
Copy link
Contributor Author

godbyk commented Jul 2, 2018

@zygoloid Well, if you have the expl3.sty file, then you should be all set. On Linux and macOS, you can run kpsewhich expl3.sty to do the search.

Or just try compiling the PDF. If it works, then you know. :-)

@Eelis Eelis mentioned this pull request Jul 3, 2018
Closed
@zygoloid zygoloid force-pushed the master branch 2 times, most recently from e3dbfe2 to 1a21a65 Compare July 7, 2018 23:19
@zygoloid
Copy link
Member

zygoloid commented Oct 8, 2018

Reviewers: please can you check whether this works for your LaTeX environment?

@tkoeppe
Copy link
Contributor

tkoeppe commented Oct 9, 2018

@godbyk: 1) please rebase, 2) please check readmes, travises, makefiles etc for whether lists of required packages need to be updated.

tkoeppe
tkoeppe reviewed Oct 9, 2018
View reviewed changes
source/compatibility.tex Outdated
@@ -1151,7 +1151,7 @@
that interacts with newer \Cpp{} code using regions declared reachable may
have different runtime behavior.

\diffrefs{refwrap}{arithmetic.operations}, \ref{comparisons},
\diffref{refwrap,arithmetic.operations,comparisons},
\ref{logical.operations}, \ref{bitwise.operations}
Copy link
Contributor

Choose a reason for hiding this comment

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

There are additional \refs here.

Please also check the other cases, I haven't looked thoroughly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I didn't notice any others.

@godbyk
Copy link
Contributor Author

godbyk commented Oct 9, 2018

@tkoeppe Done and done.

I updated the packages lists in the README by checking the contents of the packages through each distro's website. I haven't actually tested the build on each distro.

@jensmaurer jensmaurer added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Oct 11, 2018
jensmaurer
jensmaurer approved these changes Oct 11, 2018
View reviewed changes
Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Works for me.

source/macros.tex Outdated
@@ -337,8 +337,24 @@

%% Change descriptions
\newcommand{\diffdef}[1]{\hfill\break\textbf{#1:}\space}
\newcommand{\diffref}[1]{\pnum\textbf{Affected subclause:} \ref{#1}}
\newcommand{\diffrefs}[2]{\pnum\textbf{Affected subclauses:} \ref{#1}, \ref{#2}}
\RequirePackage{expl3,xparse}
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this to be \usepackage items in std.tex so that the list of required packages is clearly visible where a (LaTeX 2) person would expect it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jensmaurer Done!

@jensmaurer jensmaurer removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Oct 25, 2018
@Eelis
Copy link
Contributor

Eelis commented Nov 28, 2018

Needs rebase again I'm afraid.

@godbyk
Copy link
Contributor Author

godbyk commented Nov 28, 2018

@Eelis Is it a good time to rebase or are there a bunch more merges imminent?

@Eelis
Copy link
Contributor

Eelis commented Nov 28, 2018

I'm not sure, sorry. @zygoloid probably knows.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 28, 2018

Since the latest working paper candidate has just been created from these sources, now is probably a good time to rebase. There shouldn't be many large changes that are currently eligible for consideration.

@godbyk
Copy link
Contributor Author

godbyk commented Nov 30, 2018

@Eelis @tkoeppe Okay, I've rebased this now.

@jensmaurer
Copy link
Member

@zygoloid, can we make progress here, and either take it or leave it? (I like the regularity with the change.)

@zygoloid zygoloid merged commit a3a03ad into cplusplus:master Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkoeppe tkoeppe tkoeppe left review comments

@jensmaurer jensmaurer jensmaurer approved these changes

@jwakely jwakely Awaiting requested review from jwakely

@zygoloid zygoloid Awaiting requested review from zygoloid

@burblebee burblebee Awaiting requested review from burblebee

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@godbyk @zygoloid @tkoeppe @Eelis @jensmaurer