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 switch alternative syntax #2648

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

alexandre-daubois
Copy link
Contributor

Fix #2635

<?php case 1: ?>
...
<?php endswitch ?>
<?php endswitch; ?>
Copy link
Member

Choose a reason for hiding this comment

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

Both styles do work, but having the semicolon is probably better.

@Girgias Girgias merged commit 1d73d6e into php:master Aug 7, 2023
2 checks passed
@@ -67,9 +67,10 @@ endif;
<programlisting role="php">
<![CDATA[
<?php switch ($foo): ?>

<?php case 1: ?>
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the actual problem that the rendered version dropped the indentation of this line?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe... that's something that should be flagged on PhD (and the joy of having no tests for it to catch regressions...) hopefully the new line break will "fix" this anyway

Copy link
Member

Choose a reason for hiding this comment

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

It was a CSS issue, no white-space: pre. I also found the previous version a more "natural" mistake, because it can easily halten that someone wants to indent the next line, whereas adding an empty line is less likely.

Copy link
Member

Choose a reason for hiding this comment

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

Right, if it's fixed then please feel free to revert.

@alexandre-daubois alexandre-daubois deleted the fix-alternative-syntax branch August 8, 2023 05:43
TimWolla added a commit to php/phd that referenced this pull request Aug 10, 2023
* Drop `\n` when highlighting

Actual newlines will be replaced by `<br />` during highlighting and the `\n`
in the boilerplate conflicts with `white-space: pre;`.

see php/doc-en#2648

* Simplify character replacement for `highlight_string` result
Girgias pushed a commit to php/web-php that referenced this pull request Aug 16, 2023
This is needed to correctly handle whitespace outside of a `<?php … ?>` portion.

see b7bccc4
see php/phd@01d6beb
see php/doc-en#2648
TimWolla added a commit to TimWolla/php-doc-en that referenced this pull request Aug 18, 2023
As per the discussion in php#2648, the part that might actually be
unexpected is that leading indentation also causes problems.  The addition of
the empty line distracts from that. Now that the CSS for PHP.net is fixed, this
can be reverted.

This reverts commit 1d73d6e.

see php/web-php#801
TimWolla added a commit that referenced this pull request Aug 21, 2023
As per the discussion in #2648, the part that might actually be
unexpected is that leading indentation also causes problems.  The addition of
the empty line distracts from that. Now that the CSS for PHP.net is fixed, this
can be reverted.

This reverts commit 1d73d6e.

see php/web-php#801
TimWolla added a commit to TimWolla/php-doc-en that referenced this pull request Aug 21, 2023
This was part of an unrelated commit and is now re-added in a dedicated
single-purpose commit.

see php#2648
see php#2693
TimWolla added a commit that referenced this pull request Aug 21, 2023
This was part of an unrelated commit and is now re-added in a dedicated
single-purpose commit.

see #2648
see #2693
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.

Examples in the Warning section are the same
3 participants