[Workflow] remove new lines from workflow metadata #49272
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carsonbot
changed the title
OskarStark
reviewed
Feb 7, 2023
OskarStark
approved these changes
Feb 7, 2023
lyrixx
reviewed
Feb 7, 2023
alexislefebvre
force-pushed
the
5.4-remove-new-lines-from-workflow-metadata
branch
from
f295de1
to
f0e8f16
Compare
|
Thanks for the reviews, suggestions have been taken into account. |
OskarStark
approved these changes
Feb 8, 2023
|
I forgot to ask, are you sur other dumper needs this patch? |
|
I used the sanitizer so that the tests are not broken but I don't know if Mermaid or Graphviz accept new lines. |
|
IMHO, you should fix only what is broken ; so there is not need for the trait anymore I guess. Sorrry to go back and forth! |
|
Fair enough, let's not remove new lines if the tools support them, I'll check later. Status: Needs work |
|
It took some examples and added new lines in the transitions. Graphviz DotWith this input: Passing it to the Docker image New lines don't cause errors, and they appear in the result. ✔️ MermaidAdding a new line in any transition in the live editor results in a Parse error. I'm pretty sure that the live editor behaves like Mermaid, so I didn't test further. ❎ SummarySymfony can keep new lines in the Graphviz dumper. The other formats need new lines to be removed. |
|
Status: Needs review |
OskarStark
approved these changes
Feb 16, 2023
fabpot
reviewed
Feb 16, 2023
lyrixx
approved these changes
Feb 17, 2023
fabpot
approved these changes
Feb 20, 2023
fabpot
force-pushed
the
5.4-remove-new-lines-from-workflow-metadata
branch
from
d320dfa
to
a3062c7
Compare
|
Thank you @alexislefebvre. |
alexislefebvre
deleted the
5.4-remove-new-lines-from-workflow-metadata
branch
nicolas-grekas
added a commit
that referenced
this pull request
Feb 24, 2023
… when rendering a PUML dump (alexislefebvre) This PR was merged into the 5.4 branch. Discussion ---------- [Workflow] display label with new lines + colours properly when rendering a PUML dump | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #49311 | License | MIT | Doc PR | no In #49272 I removed the new lines instead of formatting them properly. The PUML format accept new lines in labels of transitions as long as the source file uses actual `\n` strings instead of line returns (like `PHP_EOL`). I propose a novel approach that will handle new lines with or without colours. Compare these outputs from plantuml.com, inputs are based on the [fixtures from Symfony](https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Workflow/Tests/fixtures/puml/arrow/complex-state-machine-marking.puml): ### After #49272 New lines are ignored >  ([source](https://www.plantuml.com/plantuml/uml/TOx1IWCn48RlUOeXFHTfgxR8kfIjwCbB5Jo8I3OPsx2Jf2HPj8ZlRfAwKgJc5Bx_ctzc6QBmiJV4195xVpNwGziDYpeImeCsEy8RBJPU61OwRNSY_Q2aZVCA_ThrLgsSj-XXSd7QUTngsLaC0QP7GbeS4JuPfDS8sMtyeOgShofjTTI2wXf6YtaxFv-Srepm7QfipHQB-UgoM8UbnVY77qysb4fBVkji_9i-RNL4ziKEntB1uUYsWRPy-CcS3yC3L9pbmV6upkeLy3X9H2NoF5gZUldbrLkw06G-uVhEuxw-tuFiGtG6eXSsfBNE0eaM2MRLMRRhrDIMfePwB5Moh9Z-19ceGcQSBT6gtj0t)) ### Correct output from this PR With this patch → the second line is Grey and no tag is visible >  ([source](https://www.plantuml.com/plantuml/uml/TP9DImCn48Rl-HL3UYxILcsHTIbRqPENAdXGaMmojc7pKP8iMiJ_RfBjwg9rJ-6PP-Pz3xlqWRdGQaMOKlRjHSjtQJOaoA0GxgJUARoIREEO9hwHPiVY2_AqiawWMzlMY9Lr1XrCpeuxzrl96uFUmtGWnE20y44WVXNZpSPrfvHrHI6D39AfieJHObxFJoV7DSrSWo9PiyLYlZhFLXUQZN_uSBDIyMYUNriJVayVjZ8W-IHTMSee3BhrjARzYrFuMUwXe2GjZiTbKY-0Xaaa8fB7qHh5ypSlNcC3uAd2vOt3VNcx1zxwO3K4nuoFiTOK9yagdymVMx4Q5SmEGeoeSqIbMimPF6TF3uD4H2OpIfPeHFe7lW00)) --- ### Why close and open `<font>` tags? The output is broken if we use `\n` without taking into account the `<font>` tags → there is a second line but it is black and `</font>` is visible >  ([source](https://www.plantuml.com/plantuml/uml/TP5VI_Cm5CRlyoaEsVLusVRgYjGoEj4hRwRWXOgaoR4BpP-HfEWGlxj9QgN8x5NuFR_pd0FT-C1SwBKYJ2dxzgBbkpGj2J8eX3kf3mgl96iTqyHtqXnOV45EQ-i4kftjZTXQPz31ukoqSx-Nl3FeFImdGbmS43u8nAzcl6lTKAMiAqjfP91CLHNCYdMp_hyuhMdcEXJ9MXN5UdkUhM5fDlxWqybQnTASNriJVgSFUncGV9BXMCeeJ6uRQKF75q_vE3n2GKaQdC-hf5u03Oj8H2IFinsAvnzUliOBWASJBcyS7glR8_3U1wiXE6PyN6lDar6iGGMhyb_IgrZLvAfQzPYxUDwn_0uI4PciADcW4UbVVm40)) Commits ------- 361dce2 fix style of label containing new lines in PUML dump
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Don't dump new lines in workflow dumps.