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 extra space before a where clause #95813

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Apr 8, 2022

Remove extra space before where clause in the generated documentation.

The fix is to move the space before the break-line so that it doesn't appear visually but is still here. Removing it completely would create things like this impl<D> Delta<D>where D: MyTrait (missing a space before the where) which I don't think we want.

Added two regression test, first one test that the <br> is after the space and the second check that the <br> is before the spaces.

Before:
image

After:
image

r? @GuillaumeGomez

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 8, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2022
@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 8, 2022

📌 Commit 1da3c4481904d5ce02a2bf437a7960962cc06428 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2022
@Urgau Urgau force-pushed the rustdoc-where-clause-space branch from 1da3c44 to f6c7f10 Compare April 8, 2022 15:58
@GuillaumeGomez
Copy link
Member

Ah, you forgot a fmt? 😄

@bors: r+

@GuillaumeGomez
Copy link
Member

Oh actually, let's wait for the tests to pass to be sure.

@bors: r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 8, 2022
@Urgau
Copy link
Member Author

Urgau commented Apr 8, 2022

Ah, you forgot a fmt? smile

Well I did a fmt but tidy checks everything including tests which fmt doesn't format at all.
My bad, sorry for the noise.

@GuillaumeGomez
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 8, 2022

📌 Commit f6c7f10 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 8, 2022
…=GuillaumeGomez

Remove extra space before a where clause

Remove extra space before where clause in the generated documentation.

The fix is to move the space before the break-line so that it doesn't appear visually but is still here. Removing it completely would create things like this `impl<D> Delta<D>where D: MyTrait` (missing a space before the where) which I don't think we want.

Added two regression test, first one test that the `<br>` is after the space and the second check that the `<br>` is before the spaces.

Before:
![image](https://user-images.githubusercontent.com/3616612/162475212-d4bb6727-ed66-4a55-a4a2-4f55189bf8bd.png)

After:
![image](https://user-images.githubusercontent.com/3616612/162475467-508fd082-60a7-4a8c-b693-8b188e8843e6.png)

r? `@GuillaumeGomez`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 9, 2022
…=GuillaumeGomez

Remove extra space before a where clause

Remove extra space before where clause in the generated documentation.

The fix is to move the space before the break-line so that it doesn't appear visually but is still here. Removing it completely would create things like this `impl<D> Delta<D>where D: MyTrait` (missing a space before the where) which I don't think we want.

Added two regression test, first one test that the `<br>` is after the space and the second check that the `<br>` is before the spaces.

Before:
![image](https://user-images.githubusercontent.com/3616612/162475212-d4bb6727-ed66-4a55-a4a2-4f55189bf8bd.png)

After:
![image](https://user-images.githubusercontent.com/3616612/162475467-508fd082-60a7-4a8c-b693-8b188e8843e6.png)

r? ``@GuillaumeGomez``
@Dylan-DPC
Copy link
Member

failed in rollup

@Dylan-DPC
Copy link
Member

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 9, 2022
@Urgau Urgau force-pushed the rustdoc-where-clause-space branch from f6c7f10 to 3b0f99e Compare April 9, 2022 08:07
@Urgau
Copy link
Member Author

Urgau commented Apr 11, 2022

I've opened #95933 to do comparison on the actual tree instead of the plain html.

@rustbot label -S-waiting-on-review +S-waiting-on-author +S-blocked

@rustbot rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 11, 2022
@Urgau
Copy link
Member Author

Urgau commented Apr 11, 2022

@GuillaumeGomez Any idea about how to proceed to fix the CI issue (as #95933 was rejected) ?

@rustbot label -S-blocked

@rustbot rustbot removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Apr 11, 2022
@Urgau
Copy link
Member Author

Urgau commented Apr 11, 2022

After chatting in PM on Zulip with @GuillaumeGomez we found out that the problem probably lies in

actual_str = ET.tostring(tree).decode('utf-8')
which doesn't preserve the order of the attributes leading to the attribute order changed.

We are currently investigating potential solutions.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 16, 2022
…e, r=GuillaumeGomez

htmldocck: Compare HTML tree instead of plain text html

This PR improves `htmldocck` by comparing HTML trees instead of plain text html in the case of doing a ``@snapshot`` test.

This fix the [CI issue](https://github.com/rust-lang-ci/rust/runs/5964305020?check_suite_focus=true) encounter in rust-lang#95813 where for some unknown reason one of the attributes is not always at the same place.

The code is largely based on https://github.com/formencode/formencode/blob/3a1ba9de2fdd494dd945510a4568a3afeddb0b2e/formencode/doctest_xml_compare.py#L72-L120 which is behind MIT License. The comparison function is straightforward except for the `text_compare` function which does some weird stuff that we may want to simply reduce to a plain old comparison.

r? `@GuillaumeGomez`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 16, 2022
…e, r=GuillaumeGomez

htmldocck: Compare HTML tree instead of plain text html

This PR improves `htmldocck` by comparing HTML trees instead of plain text html in the case of doing a ```@snapshot``` test.

This fix the [CI issue](https://github.com/rust-lang-ci/rust/runs/5964305020?check_suite_focus=true) encounter in rust-lang#95813 where for some unknown reason one of the attributes is not always at the same place.

The code is largely based on https://github.com/formencode/formencode/blob/3a1ba9de2fdd494dd945510a4568a3afeddb0b2e/formencode/doctest_xml_compare.py#L72-L120 which is behind MIT License. The comparison function is straightforward except for the `text_compare` function which does some weird stuff that we may want to simply reduce to a plain old comparison.

r? ``@GuillaumeGomez``
@Urgau
Copy link
Member Author

Urgau commented Apr 17, 2022

#95933 was merged. This should be ready for another re-approval.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 17, 2022
@GuillaumeGomez
Copy link
Member

Please rebase it on master so we can ensure it's working as expected. :)

@Urgau Urgau force-pushed the rustdoc-where-clause-space branch from b127f82 to f7ce145 Compare April 17, 2022 10:34
@GuillaumeGomez
Copy link
Member

Thanks for fixing the HTML checker. :)

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Apr 19, 2022

📌 Commit f7ce145 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
…=GuillaumeGomez

Remove extra space before a where clause

Remove extra space before where clause in the generated documentation.

The fix is to move the space before the break-line so that it doesn't appear visually but is still here. Removing it completely would create things like this `impl<D> Delta<D>where D: MyTrait` (missing a space before the where) which I don't think we want.

Added two regression test, first one test that the `<br>` is after the space and the second check that the `<br>` is before the spaces.

Before:
![image](https://user-images.githubusercontent.com/3616612/162475212-d4bb6727-ed66-4a55-a4a2-4f55189bf8bd.png)

After:
![image](https://user-images.githubusercontent.com/3616612/162475467-508fd082-60a7-4a8c-b693-8b188e8843e6.png)

r? `@GuillaumeGomez`
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#95740 (asm: Add a kreg0 register class on x86 which includes k0)
 - rust-lang#95813 (Remove extra space before a where clause)
 - rust-lang#96029 (Refactor loop into iterator; simplify negation logic.)
 - rust-lang#96162 (interpret: Fix writing uninit to an allocation)
 - rust-lang#96165 (Miri provenance cleanup)
 - rust-lang#96205 (Use futex locks on emscripten.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a0ba15b into rust-lang:master Apr 20, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 20, 2022
@Urgau Urgau deleted the rustdoc-where-clause-space branch May 5, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants