Fix redundant iteration while splitting lines#14720
Fix redundant iteration while splitting lines#14720hez2010 wants to merge 1 commit intoPowerShell:masterfrom
Conversation
We split a line to fit in a windows width and if on last iteration we did get zero length tail we did next iteration but we should stop.
| for (int k = 0; k < lines.Length; k++) | ||
| { | ||
| if (lines[k] == null || displayCells.Length(lines[k]) <= firstLineLen) | ||
| var currentLine = lines[k]; |
There was a problem hiding this comment.
| var currentLine = lines[k]; | |
| string currentLine = lines[k]; |
We should respect EditorConfig setting: csharp_style_var_for_built_in_types = false.
| @@ -87,8 +87,15 @@ internal DisplayCellsPSHost(PSHostRawUserInterface rawUserInterface) | |||
|
|
|||
| internal override int Length(string str, int offset) | |||
| { | |||
There was a problem hiding this comment.
Are the changes to this method necessary to fix the issue?
There was a problem hiding this comment.
No. But it's a refactor to Length to reduce ops while str is empty.
|
|
||
| if (offset < 0 || offset >= str.Length) | ||
| { | ||
| throw PSTraceSource.NewArgumentException(nameof(offset)); |
There was a problem hiding this comment.
In particular this exception.
|
@iSazonov can I push to your branch? or I have to create another PR from my fork and close this one. |
It is better to use your fork if you want to add something. Please avoid in-depth refactoring. |
|
I'm closing this PR and create another one from my folk later. |
PR Summary
Fix redundant iteration while splitting lines to prevent assert failure.
PR Context
We split a line to fit in a windows width and if on last iteration we did get zero length tail we did next iteration but we should stop.
Borrowed from @iSazonov, see #13551 (comment)
Fixes #13551
Todo:
[ ] Add tests: can only repro in a specified range of width of window while rendering table, I don't know how to test it automatically without user interaction.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.