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

Report Full: various message formatting fixes #2093

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 15, 2018

This patch contains three commits addressing separate issues related to the Full report message formatting.

Report Full: fix word wrapping when message contains new lines

The word wrapping was causing unexpected and unintended message formatting if the message contained new lines.

Looking at the file history, prior to PHPCS 2.4.0 line breaks in messages wasn't supported at all. The implementation in PHPCS 2.4.0, however, pads the lines before the word wrapping causing the word wrapping to go haywire.

Test code:

$message = "Lorem ipsum dolor sit amet, consectetur adipiscing elit.\nAenean felis urna, dictum vitae lobortis vitae, maximus nec enim. Etiam euismod placerat efficitur. Nulla eu felis ipsum.\nCras vitae ultrices turpis. Ut consectetur ligula in justo tincidunt mattis.\n\nAliquam fermentum magna id venenatis placerat. Curabitur lobortis nulla sit amet consequat fermentum. Aenean malesuada tristique aliquam. Donec eget placerat nisl.\n\nMorbi mollis, risus vel venenatis accumsan, urna dolor faucibus risus, ut congue purus augue vel ipsum.\nCurabitur nec dolor est. Suspendisse nec quam non ligula aliquam tempus. Donec laoreet maximus leo, in eleifend odio interdum vitae.";

$phpcsFile->addWarning($message, 0, 'Example');

What it used to look like:

phpcs ./file.php --standard=Standard

messagewrappinglinebreaks-old-without-errorcode

phpcs -s ./file.php --standard=Standard (with show sources enabled)

messagewrappinglinebreaks-old-with-errorcode

What it looks like with this patch applied:

phpcs ./file.php --standard=Standard

messagewrappinglinebreaks-new-without-errorcode

phpcs -s ./file.php --standard=Standard (with show sources enabled)

messagewrappinglinebreaks-new-with-errorcode

## Report Full: always have the error code on a new line

In most cases it already would be as the error code is often long, but sometimes it wouldn't be, causing decreased readability of the report.

Report Full: fix max report width not being respected when showing sources

The $maxErrorSpace was being given a extra width of 8 when $showSources was true - i.e. when -s was used -, causing the report to be wider than the report-width setting allowed.

Test code:

$message = "Lorem ipsum dolor sit amet, consectetur adipiscing elit.\nAenean felis urna, dictum vitae lobortis vitae, maximus nec enim. Etiam euismod placerat efficitur. Nulla eu felis ipsum.\nCras vitae ultrices turpis. Ut consectetur ligula in justo tincidunt mattis.\n\nAliquam fermentum magna id venenatis placerat. Curabitur lobortis nulla sit amet consequat fermentum. Aenean malesuada tristique aliquam. Donec eget placerat nisl.\n\nMorbi mollis, risus vel venenatis accumsan, urna dolor faucibus risus, ut congue purus augue vel ipsum.\nCurabitur nec dolor est. Suspendisse nec quam non ligula aliquam tempus. Donec laoreet maximus leo, in eleifend odio interdum vitae.";
$message  = str_replace("\n", ' ', $message);
$message  = str_replace('  ', ' ', $message);

$phpcsFile->addWarning($message, 0, 'Example');

What it used to look like:

messagewrappinglinebreaks-old-sources-versus-no-sources

What it looks like with this patch applied:

messagewrappinglinebreaks-new-sources-versus-no-sources

The word wrapping was causing unexpected and unintended message formatting if the message contained new lines.
@gsherwood
Copy link
Member

I haven't had time to look at this yet (not sure when) but wanted to comment to say that changing the full report to always put sources on a new line would be a real pain for me during development (I have plenty of width, not much height) and I would not merge in this as the default option for the report. Those sources have always been on the same line, so I imagine I'm not the only one who would be annoyed by that change.

FYI, my terminal width normally sits at around 160 and most errors and sources fit on one line. If you're at something more like 100, then yes, you're going to be on new lines most of the time, but I don't think it matters that the source stays on the same line if possible.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 16, 2018

@gsherwood And that's why the three changes are all separate commits. 😎 I'll remove that specific commit.

@jrfnl jrfnl force-pushed the feature/report-full-fix-message-with-linebreaks branch from 0028bcb to 4a46c83 Compare July 16, 2018 00:49
@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 16, 2018

@gsherwood FYI: I've removed the commit adding the line break before the error code and have adjusted the last commit (max report width not being respected) to be in line with that change not being made.

The last line of a long wrapped message may wrap four characters early now, but in my opinion, that's better than the whole message being too wide.

@gsherwood gsherwood added this to the 3.3.1 milestone Jul 25, 2018
@gsherwood gsherwood added this to Backlog in PHPCS v3 Development via automation Jul 25, 2018
@gsherwood gsherwood moved this from Backlog to In Progress in PHPCS v3 Development Jul 25, 2018
@gsherwood gsherwood merged commit 4a46c83 into squizlabs:master Jul 25, 2018
gsherwood added a commit that referenced this pull request Jul 25, 2018
PHPCS v3 Development automation moved this from In Progress to Ready for Release Jul 25, 2018
@gsherwood
Copy link
Member

Thanks for these fixes, and for removing that error code line change.

@jrfnl jrfnl deleted the feature/report-full-fix-message-with-linebreaks branch July 25, 2018 05:51
@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 25, 2018

Thanks for merging this.

jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Apr 5, 2021
This class initially introduces four new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Apr 5, 2021
This class initially introduces four new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request Apr 5, 2021
This class initially introduces four new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request May 12, 2021
This class initially introduces four new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request May 12, 2021
This class initially introduces five new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request May 12, 2021
This class initially introduces five new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request May 12, 2021
This class initially introduces five new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
jrfnl added a commit to PHPCSStandards/PHPCSUtils that referenced this pull request May 12, 2021
This class initially introduces five new utility methods for working with error/warning messages.

The class currently contains the following methods:
* `addMessage()` - simple method to add either an error or a warning to PHPCS based on an `$isError` parameter. Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `addFixableMessage()` - simple method to add either a fixable error or a fixable warning to PHPCS based on an `$isError` parameter.  Returns boolean (same as PHPCS natively).
    Supports all optional parameters supported by PHPCS.
* `stringToErrorcode()` - to convert an arbitrary text string to an alphanumeric string with underscores. Returns the adjusted text string.
    This method is intended to pre-empt issues in XML and PHP when arbitrary text strings are used as (part of) an error code.
* `hasNewLineSupport()` - to check whether PHPCS can properly handle new lines in violation messages.
    Prior to PHPCS 3.3.1, new line support in error messages was buggy.
    Ref: squizlabs/PHP_CodeSniffer#2093
* `showEscapeChars()` - to make the whitespace escape codes used in an arbitrary text string visible. Returns string.

Includes dedicated unit tests for each method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PHPCS v3 Development
Ready for Release
Development

Successfully merging this pull request may close these issues.

None yet

2 participants