8000 [Console] Formatting breaks on new line · Issue #19172 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Formatting breaks on new line #19172

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.

Sign up for GitHub

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

Jump to bottom
Closed
ro0NL opened this issue Jun 24, 2016 · 11 comments
Closed

[Console] Formatting breaks on new line #19172

ro0NL opened this issue Jun 24, 2016 · 11 comments
Labels
Bug Console Status: Needs Review

Comments

@ro0NL
Copy link
Contributor
ro0NL commented Jun 24, 2016 •
edited
Loading

Hi,

Formatting seems to break on new lines.

image

// on the last line should be white
@chalasr
Copy link
Member
chalasr commented Jun 24, 2016 •
edited
Loading

Hi @ro0NL.

Do you get this output by calling SymfonyStyle::comment() with an array of strings as argument?

I think the issue would always happen when using SymfonyStyle::block with a prefix.
We should be able to fix this bug by excluding the prefix ($prefix argument of SymfonyStyle::block, set as // for comments). I'll investigate and keep you informed.

@chalasr
Copy link
Member
chalasr commented Jun 24, 2016 •
edited
Loading

It only happens on comment() since messages are formatted and use block() with a prefix (made in #18889). For now I don't see any solution since the console colours whatever char between the opening tag and the closing one. A workaround is too manually break lines by passing an array of strings rather than a long string then cut the tag for each line, but it's very not a real solution...

@ro0NL
Copy link
Contributor Author
ro0NL commented Jun 25, 2016 •
edited
Loading

Hi,

I checked it last night.. and this can get really complex :-)

My thoughts so far;-

  • Fix/support it at block() level for efficiency, comment() should be just a proxy method
  • Test each line for a (partial) opening tag (before/after formatting) without a closing tag
  • Get next lines by reference until closing tag is found
  • Fix the formatting for the starting/ending line (ie finalize and close formatting tags per line as this doesnt affect line length visually)

Another solution could be to capture all string offsets for tags, remove them, wrap etc., recalculate offsets and replace/format them manually afterwards.

Or match broken tags after wrapping and before formatting, ie <[^>]*'.preg_quote(PHP_EOL.$prefix.PHP_EOL).'[^>]*>. Fix, then format.

This targets 2.7 btw, but i think we cant change block() then due BC. So i propose master :-)
Seems formatting for block() is supported (i.e. writeln already has built-in formatting) but is terribly broken right now :)

@ro0NL
Copy link
Contributor Author
ro0NL commented Jun 25, 2016 •
edited
Loading

To clearify, i removed formatting at comment() level and expected the 2nd line to be formatted due writeln

// Provide the name of a bundle as the first argument of this command to dump its configuration. (e.g. 
// <comment>debug:config FrameworkBundle</comment>)

However escaping kicks in first.

@chalasr
Copy link
Member
chalasr commented Jun 25, 2016

Seems formatting for block() is supported (i.e. writeln already has built-in formatting) but is terribly broken right now :)

Not sure but, even if it calls writeln() at the end, I think that block() doesn't and shouldn't support formatting through tags, the style needs to be passed as third argument and format the whole block based on, that's why messages are first escaped.

Or match broken tags after wrapping and before formatting, ie <[^>]'.preg_quote(PHP_EOL.$prefix.PHP_EOL).'[^>]>. Fix, then format.

Hmm. Did you tried something based on this one or another alternative you listed?
IMO without going further, an eventual fix would be effectively complex.
I'll give it some tries (based on what you said) today.

@ro0NL
Copy link
Contributor Author
ro0NL commented Jun 25, 2016 •
edited
Loading

From a formatting pov it does work, ie it could work for block

$io->writeln('<fg=black;bg=green>foo <comment>bar</comment> baz</>');
image

I think that block() doesn't and shouldn't support formatting through tags

Well.. it's being used right now :) i guess people, at least i, also expect it to work. The thing is, is it worth the effort. Disallowing means undoing formatting in comment() and update messages as such.

IMO without going further, an eventual fix would be effectively complex.

Yes :)

@ro0NL
Copy link
Contributor Author
ro0NL commented Jun 25, 2016 •
edited
Loading

Also the formatting logic is tight to a concrete formatter, ie we cannot fix this without changing its api. Ie. it needs to be fixed via format($message, $wrap = null). Which perhaps makes it a bit less complex.

@chalasr
Copy link
Member
chalasr commented Jun 25, 2016

We effectively have some work to do for nested tags in block()...
Tags need to be parsed, striped and properly re-added before/while cutting lines to the correct width.

Actually if I comment out the escaping line, calling this:

$io->block('Lorem <comment> ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</comment>', null, 'fg=black;bg=green');

Leads to

Fatal error: Uncaught Symfony\Component\Console\Exception\InvalidArgumentException: Incorrectly nested style tag found. in /Volumes/HD/component-alone/Console/vendor/symfony/console/Formatter/OutputFormatterStyleStack.php:87

IMHO the problem is too specific to block() for doing something on the formatter side.
I would go more for moving the line-cutting logic from block() to a separate utility method then calling this method in comment(), rather than using block() directly.
It would help to fix the first problem pointed here (nested tag covering multiple lines of ), letting block() escaping messages as it actually does.

@ro0NL
Copy link
Contributor Author
ro0NL commented Jun 25, 2016 •
edited
Loading

My main concern is we create a performance killer when doing it in multiple steps. For instance Helper::strlenWithoutDecoration is already overkill.

I still think it belongs on the formatter and/or formatter-style side, from a OO pov. I.e. strlenWithoutDecoration also belongs their ($formatter->strlen()).

@chalasr
Copy link
Member
chalasr commented Jun 25, 2016

Actually, commenting out the escaping line and calling this:

$io->block('Lorem <comment> ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</comment>', null, 'fg=black;bg=green');

Leads to

Fatal error: Uncaught Symfony\Component\Console\Exception\InvalidArgumentException: Incorrectly nested style tag found. in /Volumes/HD/component-alone/Console/vendor/symfony/console/Formatter/OutputFormatterStyleStack.php:87

IMHO the problem is too specific to block() for doing something on the formatter side.
I would go more for moving the line-cutting logic from block() to a separate utility method then call this method from comment() and block(), rather than using block() directly from comment().
It would help for fixing the first problem pointed here (nested tag covering multiple lines in comment()), letting block() escaping messages as it actually does.

Output formatting becomes a real headache here.
Personally I wouldn't be happy to get this kind of output when using block():

IMHO the SymfonyStyle should primary stay a set of utility methods providing an easy way to have formatted blocks based on a fixed style.
The problem that stays here is that calling block() from comment() is not compatible because nested tags should be allowed in comment() and not in block() (IMHO again).

Let's wait for the opinion of maintainers.

@ro0NL
Copy link
Contributor Author
ro0NL commented Jun 25, 2016 •
edited
Loading

👍 I understand your pov. I guess i just dont like the OutputFormatterInterface with tight coupling everywhere (i.e. also OutputFormatter::escape is problematic).

edit:
Also see #18976 and #19180

@chalasr chalasr mentioned this issue Jun 26, 2016
Merged
fabpot added a commit that referenced this issue Jun 29, 2016
@fabpot
bug #19189 [Console] Fix formatting of SymfonyStyle::comment() (chalasr)
c278399 
This PR was merged into the 2.8 branch.

Discussion
----------

[Console] Fix formatting of SymfonyStyle::comment()

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19172
| License       | MIT
| Doc PR        | n/a

This:

```php
$io->comment('Lorem ipsum dolor sit amet, consectetur adipisicing elit, <comment>sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat </comment>cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.');
```

Before outputs:

![](http://image.prntscr.com/image/1d2ea9de42024b53a77120c482be51d4.png)

After:

![](http://image.prntscr.com/image/36de23ec14b64804b0cbae7a431185be.png)

This moves the lines-cutting logic from `block()` into a specific `createBlock`, used from both `comment()` and `block()`, sort as `comment()` can take messages containing nested tags and outputs a correctly formatted block without escaping tags.

Commits
-------

0a53e1d [Console] Fix formatting of SymfonyStyle::comment()
@fabpot fabpot closed this as completed Jun 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Console Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @javiereguiluz @ro0NL @chalasr @carsonbot
0