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

[Merged by Bors] - fix(widget): workaround for webview rendering bug #3997

Closed
wants to merge 3 commits into from

Conversation

EdAyers
Copy link
Member

@EdAyers EdAyers commented Aug 31, 2020

See https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/extension.20performance
The bug seems to go away if we collapse the extra nested spans made by block in to one span.

Still should do some tests to make sure this doesn't break anything else.

Minimal breaking example is:

import tactic.interactive_expr

example :
0+1+2+3+4+5+6+7+8+9 +
0+1+2+3+4+5+6+7+8+9 =
0+1+2+3+4+5+6+7+8+9 :=
by skip

See https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/extension.20performance
The bug seems to go away if we collapse the extra nested spans made by `block` in to one span.

Still should do some tests to make sure this doesn't break anything else.

Minimal breaking example is:

```
import tactic.interactive_expr

example :
0+1+2+3+4+5+6+7+8+9 +
0+1+2+3+4+5+6+7+8+9 =
0+1+2+3+4+5+6+7+8+9 :=
by skip
```
Copy link
Member

@gebner gebner left a comment

Choose a reason for hiding this comment

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

Feel free to merge when you're done with testing.

bors d+

("white-space", "pre-wrap"),
("vertical-align", "top")
],
(a,rest) ← get_block_attrs a,
Copy link
Member

Choose a reason for hiding this comment

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

A related question: does
[style [("x", "y")], className "a", style [("z", "w")], className "b"]
do the right thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it does

Copy link
Member Author

Choose a reason for hiding this comment

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

it does a merge of the styles

Copy link
Member Author

Choose a reason for hiding this comment

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

and classNames

Copy link
Member Author

Choose a reason for hiding this comment

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

So it's equal to [style [("x","y"), ("z","w")], className "a b"] when it gets emitted.

@bors
Copy link

bors bot commented Aug 31, 2020

✌️ EdAyers can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@EdAyers EdAyers requested a review from gebner August 31, 2020 14:49
@EdAyers
Copy link
Member Author

EdAyers commented Aug 31, 2020

Sorry for requesting a review I didn't refresh

@gebner gebner marked this pull request as ready for review August 31, 2020 14:54
@EdAyers
Copy link
Member Author

EdAyers commented Aug 31, 2020

Ok I can't find any awful new bugs.

Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
@EdAyers
Copy link
Member Author

EdAyers commented Aug 31, 2020

bors r+

@bryangingechen
Copy link
Collaborator

Not sure if it's a github issue or if maybe the draft status confused bors. Let's try again:
bors r+

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Aug 31, 2020
bors bot pushed a commit that referenced this pull request Aug 31, 2020
See https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/extension.20performance
The bug seems to go away if we collapse the extra nested spans made by `block` in to one span.

Still should do some tests to make sure this doesn't break anything else.

Minimal breaking example is:

```
import tactic.interactive_expr

example :
0+1+2+3+4+5+6+7+8+9 +
0+1+2+3+4+5+6+7+8+9 =
0+1+2+3+4+5+6+7+8+9 :=
by skip
```




Co-authored-by: Ed Ayers <EdAyers@users.noreply.github.com>
@bors
Copy link

bors bot commented Aug 31, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix(widget): workaround for webview rendering bug [Merged by Bors] - fix(widget): workaround for webview rendering bug Aug 31, 2020
@bors bors bot closed this Aug 31, 2020
@bors bors bot deleted the widget-hotfix branch August 31, 2020 18:48
robertylewis pushed a commit that referenced this pull request Aug 31, 2020
See https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/extension.20performance
The bug seems to go away if we collapse the extra nested spans made by `block` in to one span.

Still should do some tests to make sure this doesn't break anything else.

Minimal breaking example is:

```
import tactic.interactive_expr

example :
0+1+2+3+4+5+6+7+8+9 +
0+1+2+3+4+5+6+7+8+9 =
0+1+2+3+4+5+6+7+8+9 :=
by skip
```




Co-authored-by: Ed Ayers <EdAyers@users.noreply.github.com>
PatrickMassot pushed a commit that referenced this pull request Sep 8, 2020
See https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/extension.20performance
The bug seems to go away if we collapse the extra nested spans made by `block` in to one span.

Still should do some tests to make sure this doesn't break anything else.

Minimal breaking example is:

```
import tactic.interactive_expr

example :
0+1+2+3+4+5+6+7+8+9 +
0+1+2+3+4+5+6+7+8+9 =
0+1+2+3+4+5+6+7+8+9 :=
by skip
```




Co-authored-by: Ed Ayers <EdAyers@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants