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

Page breaks (tkoeppe) #4233

Merged
merged 8 commits into from Oct 2, 2020
Merged

Page breaks (tkoeppe) #4233

merged 8 commits into from Oct 2, 2020

Conversation

tkoeppe
Copy link
Contributor

@tkoeppe tkoeppe commented Sep 24, 2020

Partially addresses #4228

@tkoeppe tkoeppe force-pushed the verticalreview branch 8 times, most recently from bc62d8a to 0208a91 Compare September 29, 2020 19:11
@tkoeppe tkoeppe changed the title Page breaks Page breaks (tkoeppe) Sep 30, 2020
@zygoloid zygoloid added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Sep 30, 2020
@zygoloid
Copy link
Member

Do you think we should insert a page break before [lex.literal]?

Other than that, looks good to the end of Clause 6.

[iterator.concept.sentinel]: wrapping of sentinel_for is unfortunate. But maybe there's nothing we can do about that without violating local consistency. It's not terrible.

Otherwise, Clause 23 and 24 look OK.

C.5.4 [diff.expr] paragraph 1 has too much vertical whitespace after the second codeblock. Same thing in C.5.2/3 and C.5.3/1 and ... looks like we get bonus vertical space everywhere that a codeblock is followed by one of our bold Annex C labels.

Otherwise, Annexes look OK.

Back matter: split_view::inner-iterator and ::outer-iterator are in the wrong typeface in the index of library names.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Sep 30, 2020

Page break added before [lex.literal].

Annex C.5: The extraneous whitespace is a problem. We should try to fix that and re-review. [#4254, applied, needs re-review]

Back matter split view: Fixing separately, #4255.

sentinel-for: see below.

@tkoeppe tkoeppe removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Sep 30, 2020
@tkoeppe
Copy link
Contributor Author

tkoeppe commented Sep 30, 2020

The sentinel-for awkwardness was fixed with a single, local \newpage, no changes outside the two affected pages.

PTAL.

Copy link
Member

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

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

Thanks, looks great!

@zygoloid zygoloid merged commit ffa073b into cplusplus:c++20 Oct 2, 2020
@tkoeppe tkoeppe deleted the verticalreview branch November 25, 2020 12:02
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.

None yet

2 participants