Fix redundant iteration while splitting lines by hez2010 · Pull Request #14851 · PowerShell/PowerShell

Steve (hez2010) · 2021-02-19T06:07:57Z

PR Summary

Fix redundant iteration while splitting lines to prevent assert failure.

Reopened PR #14720 from different folk.

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 Ilya (@iSazonov), see #13551 (comment)

Fixes #13551

PR Checklist

Ilya (iSazonov) · 2021-02-19T13:30:03Z

Steve (@hez2010) Please try to rebase from your branch

get pull --rebase PowerShell master

Steve (hez2010) · 2021-02-19T13:50:13Z

Ilya (@iSazonov) Done

Deleted user (ghost) · 2021-02-27T02:00:08Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Steve (hez2010) · 2021-02-28T07:06:36Z

Any updates on this PR?

Ilya (iSazonov) · 2021-02-28T15:44:04Z

Steve (@hez2010) Please rebase to fix CI builds.

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.

Steve (hez2010) · 2021-03-01T05:39:42Z

Ilya (@iSazonov) Done. I think CI failures are unrelated to this PR.

Ilya (iSazonov) · 2021-03-01T06:17:06Z

CI fails on https://github.com/dotnet/cli - the repo was move to https://github.com/dotnet/sdk.
There are 3 uses of the link in our md files.

GitHub
dotnet/cli
The .NET Core command-line (CLI) tools, used for building .NET Core apps and libraries through your development flow (compiling, NuGet package management, running, testing, ...). - dotnet/cli

GitHub
dotnet/sdk
Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI - dotnet/sdk

Co-authored-by: Robert Holt <[email protected]>

Steve (hez2010) · 2021-03-10T05:34:22Z

Ping! Can this PR be merged? This issue blocks me to use PowerShell in some scenarios because PowerShell will crash without this PR.

Ilya (iSazonov) · 2021-03-10T05:38:40Z

/azp run PowerShell-CI-static-analysis

azure-pipelines · 2021-03-10T05:38:51Z

Azure Pipelines successfully started running 1 pipeline(s).

Ilya (iSazonov) · 2021-03-10T05:45:16Z

Rob Holt (@rjmholt) I believe we need to fix internal override int Length(string str, int offset) too (see my commit iSazonov@6a9bf10) Otherwise the PR is not full fix for #13551.

Steve (hez2010) · 2021-03-15T15:49:56Z

Ilya (@iSazonov) I've added the fix back.

xtqqczze · 2021-03-16T23:44:15Z

        {
-            Dbg.Assert(offset >= 0, "offset >= 0");
-            Dbg.Assert(string.IsNullOrEmpty(str) || (offset < str.Length), "offset < str.Length");
+            if (string.IsNullOrEmpty(str))


Ilya (@iSazonov) Why are changes to this method necessary to fix the issue? In particular do we need replace assert with exception in this internal method?

The change is not direct fix for the issue. It is related the issue. It makes the code more reliable.
If we don't change the method we will have different behavior for debug and release code that might confuse.

Deleted user (ghost) · 2021-03-24T14:00:12Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Steve (hez2010) · 2021-03-26T11:03:47Z

Any updates?

Ilya (iSazonov) · 2021-03-27T06:01:59Z

Rob Holt (@rjmholt) Do you ready to merge?

Deleted user (ghost) · 2021-04-03T14:00:12Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Steve (hez2010) · 2021-04-05T03:35:17Z

Can someone take a look at this PR?

Ilya (iSazonov) · 2021-04-13T04:05:40Z

Steve (@hez2010) Thanks for you contribution!

Deleted user (ghost) · 2021-04-14T23:59:18Z

🎉v7.2.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

@davidBar-On

[7.2.0-preview.5] - 2021-04-14 * Breaking Changes - Make PowerShell Linux deb and RPM packages universal (#15109) - Enforce AppLocker Deny configuration before Execution Policy Bypass configuration (#15035) - Disallow mixed dash and slash in command line parameter prefix (#15142) (Thanks @davidBar-On!) * Experimental Features - `PSNativeCommandArgumentPassing`: Use `ArgumentList` for native executable invocation (breaking change) (#14692) * Engine Updates and Fixes - Add `IArgumentCompleterFactory` for parameterized `ArgumentCompleters` (#12605) (Thanks @powercode!) * General Cmdlet Updates and Fixes - Fix SSH remoting connection never finishing with misconfigured endpoint (#15175) - Respect `TERM` and `NO_COLOR` environment variables for `$PSStyle` rendering (#14969) - Use `ProgressView.Classic` when Virtual Terminal is not supported (#15048) - Fix `Get-Counter` issue with `-Computer` parameter (#15166) (Thanks @krishnayalavarthi!) - Fix redundant iteration while splitting lines (#14851) (Thanks @hez2010!) - Enhance `Remove-Item -Recurse` to work with OneDrive (#14902) (Thanks @iSazonov!) - Change minimum depth to 0 for `ConvertTo-Json` (#14830) (Thanks @kvprasoon!) - Allow `Set-Clipboard` to accept empty string (#14579) - Turn on and off `DECCKM` to modify keyboard mode for Unix native commands to work correctly (#14943) - Fall back to `CopyAndDelete()` when `MoveTo()` fails due to an `IOException` (#15077) * Code Cleanup <details> <summary> We thank the following contributors! @xtqqczze, @iSazonov, @ZhiZe-ZG </summary> <ul> <li>Update .NET to <code>6.0.0-preview.3</code> (#15221)</li> <li>Add space before comma to hosting test to fix error reported by <code>SA1001</code> (#15224)</li> <li>Add <code>SecureStringHelper.FromPlainTextString</code> helper method for efficient secure string creation (#14124) (Thanks @xtqqczze!)</li> <li>Use static lambda keyword (#15154) (Thanks @iSazonov!)</li> <li>Remove unnecessary <code>Array</code> -> <code>List</code> -> <code>Array</code> conversion in <code>ProcessBaseCommand.AllProcesses</code> (#15052) (Thanks @xtqqczze!)</li> <li>Standardize grammar comments in Parser.cs (#15114) (Thanks @ZhiZe-ZG!)</li> <li>Enable <code>SA1001</code>: Commas should be spaced correctly (#14171) (Thanks @xtqqczze!)</li> <li>Refactor <code>MultipleServiceCommandBase.AllServices</code> (#15053) (Thanks @xtqqczze!)</li> </ul> </details> * Tools - Use Unix line endings for shell scripts (#15180) (Thanks @xtqqczze!) * Tests - Add the missing tag in Host Utilities tests (#14983) - Update `copy-props` version in `package.json` (#15124) * Build and Packaging Improvements <details> <summary> We thank the following contributors! @JustinGrote </summary> <ul> <li>Fix <code>yarn-lock</code> for <code>copy-props</code> (#15225)</li> <li>Make package validation regex accept universal Linux packages (#15226)</li> <li>Bump NJsonSchema from 10.4.0 to 10.4.1 (#15190)</li> <li>Make MSI and EXE signing always copy to fix daily build (#15191)</li> <li>Sign internals of EXE package so that it works correctly when signed (#15132)</li> <li>Bump Microsoft.NET.Test.Sdk from 16.9.1 to 16.9.4 (#15141)</li> <li>Update daily release tag format to work with new Microsoft Update work (#15164)</li> <li>Feature: Add Ubuntu 20.04 Support to install-powershell.sh (#15095) (Thanks @JustinGrote!)</li> <li>Treat rebuild branches like release branches (#15099)</li> <li>Update WiX to 3.11.2 (#15097)</li> <li>Bump NJsonSchema from 10.3.11 to 10.4.0 (#15092)</li> <li>Allow patching of preview releases (#15074)</li> <li>Bump Newtonsoft.Json from 12.0.3 to 13.0.1 (#15084, #15085)</li> <li>Update the <code>minSize</code> build package filter to be explicit (#15055)</li> <li>Bump NJsonSchema from 10.3.10 to 10.3.11 (#14965)</li> </ul> </details> * Documentation and Help Content - Merge `7.2.0-preview.4` changes to master (#15056) - Update `README` and `metadata.json` (#15046) - Fix broken links for `dotnet` CLI (#14937) [7.2.0-preview.5]: v7.2.0-preview.4...v7.2.0-preview.5

@davidBar-On

[7.2.0-preview.5] - 2021-04-14 * Breaking Changes - Make PowerShell Linux deb and RPM packages universal (PowerShell#15109) - Enforce AppLocker Deny configuration before Execution Policy Bypass configuration (PowerShell#15035) - Disallow mixed dash and slash in command line parameter prefix (PowerShell#15142) (Thanks @davidBar-On!) * Experimental Features - `PSNativeCommandArgumentPassing`: Use `ArgumentList` for native executable invocation (breaking change) (PowerShell#14692) * Engine Updates and Fixes - Add `IArgumentCompleterFactory` for parameterized `ArgumentCompleters` (PowerShell#12605) (Thanks @powercode!) * General Cmdlet Updates and Fixes - Fix SSH remoting connection never finishing with misconfigured endpoint (PowerShell#15175) - Respect `TERM` and `NO_COLOR` environment variables for `$PSStyle` rendering (PowerShell#14969) - Use `ProgressView.Classic` when Virtual Terminal is not supported (PowerShell#15048) - Fix `Get-Counter` issue with `-Computer` parameter (PowerShell#15166) (Thanks @krishnayalavarthi!) - Fix redundant iteration while splitting lines (PowerShell#14851) (Thanks @hez2010!) - Enhance `Remove-Item -Recurse` to work with OneDrive (PowerShell#14902) (Thanks @iSazonov!) - Change minimum depth to 0 for `ConvertTo-Json` (PowerShell#14830) (Thanks @kvprasoon!) - Allow `Set-Clipboard` to accept empty string (PowerShell#14579) - Turn on and off `DECCKM` to modify keyboard mode for Unix native commands to work correctly (PowerShell#14943) - Fall back to `CopyAndDelete()` when `MoveTo()` fails due to an `IOException` (PowerShell#15077) * Code Cleanup <details> <summary> We thank the following contributors! @xtqqczze, @iSazonov, @ZhiZe-ZG </summary> <ul> <li>Update .NET to <code>6.0.0-preview.3</code> (PowerShell#15221)</li> <li>Add space before comma to hosting test to fix error reported by <code>SA1001</code> (PowerShell#15224)</li> <li>Add <code>SecureStringHelper.FromPlainTextString</code> helper method for efficient secure string creation (PowerShell#14124) (Thanks @xtqqczze!)</li> <li>Use static lambda keyword (PowerShell#15154) (Thanks @iSazonov!)</li> <li>Remove unnecessary <code>Array</code> -> <code>List</code> -> <code>Array</code> conversion in <code>ProcessBaseCommand.AllProcesses</code> (PowerShell#15052) (Thanks @xtqqczze!)</li> <li>Standardize grammar comments in Parser.cs (PowerShell#15114) (Thanks @ZhiZe-ZG!)</li> <li>Enable <code>SA1001</code>: Commas should be spaced correctly (PowerShell#14171) (Thanks @xtqqczze!)</li> <li>Refactor <code>MultipleServiceCommandBase.AllServices</code> (PowerShell#15053) (Thanks @xtqqczze!)</li> </ul> </details> * Tools - Use Unix line endings for shell scripts (PowerShell#15180) (Thanks @xtqqczze!) * Tests - Add the missing tag in Host Utilities tests (PowerShell#14983) - Update `copy-props` version in `package.json` (PowerShell#15124) * Build and Packaging Improvements <details> <summary> We thank the following contributors! @JustinGrote </summary> <ul> <li>Fix <code>yarn-lock</code> for <code>copy-props</code> (PowerShell#15225)</li> <li>Make package validation regex accept universal Linux packages (PowerShell#15226)</li> <li>Bump NJsonSchema from 10.4.0 to 10.4.1 (PowerShell#15190)</li> <li>Make MSI and EXE signing always copy to fix daily build (PowerShell#15191)</li> <li>Sign internals of EXE package so that it works correctly when signed (PowerShell#15132)</li> <li>Bump Microsoft.NET.Test.Sdk from 16.9.1 to 16.9.4 (PowerShell#15141)</li> <li>Update daily release tag format to work with new Microsoft Update work (PowerShell#15164)</li> <li>Feature: Add Ubuntu 20.04 Support to install-powershell.sh (PowerShell#15095) (Thanks @JustinGrote!)</li> <li>Treat rebuild branches like release branches (PowerShell#15099)</li> <li>Update WiX to 3.11.2 (PowerShell#15097)</li> <li>Bump NJsonSchema from 10.3.11 to 10.4.0 (PowerShell#15092)</li> <li>Allow patching of preview releases (PowerShell#15074)</li> <li>Bump Newtonsoft.Json from 12.0.3 to 13.0.1 (PowerShell#15084, PowerShell#15085)</li> <li>Update the <code>minSize</code> build package filter to be explicit (PowerShell#15055)</li> <li>Bump NJsonSchema from 10.3.10 to 10.3.11 (PowerShell#14965)</li> </ul> </details> * Documentation and Help Content - Merge `7.2.0-preview.4` changes to master (PowerShell#15056) - Update `README` and `metadata.json` (PowerShell#15046) - Fix broken links for `dotnet` CLI (PowerShell#14937) [7.2.0-preview.5]: PowerShell/PowerShell@v7.2.0-preview.4...v7.2.0-preview.5

ghost assigned Rob Holt (rjmholt) Feb 19, 2021

Rob Holt (rjmholt) requested a review from Steve Lee (SteveL-MSFT) February 19, 2021 18:03

ghost added the Review - Needed The PR is being reviewed label Feb 27, 2021

ghost removed the Review - Needed The PR is being reviewed label Feb 28, 2021

Ilya (iSazonov) and others added 2 commits March 1, 2021 11:54

Fix GenerateLinesWithoutWordWrap()

c102315

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.

Work with review feedback

0bc0827

Rob Holt (rjmholt) requested review from James Truher (JamesWTruher) and Dongbo Wang (daxian-dbw) March 1, 2021 19:32

xtqqczze approved these changes Mar 2, 2021

View reviewed changes

Rob Holt (rjmholt) reviewed Mar 2, 2021

View reviewed changes

Comment thread src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs Outdated

Apply changes based on review feedback

892491c

Co-authored-by: Robert Holt <[email protected]>

Rob Holt (rjmholt) approved these changes Mar 3, 2021

View reviewed changes

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Mar 10, 2021

Fix Length(string, int) override implementation

d1f3d7a

xtqqczze reviewed Mar 16, 2021

View reviewed changes

ghost added the Review - Needed The PR is being reviewed label Mar 24, 2021

Ilya (iSazonov) approved these changes Mar 27, 2021

View reviewed changes

ghost removed the Review - Needed The PR is being reviewed label Mar 27, 2021

ghost added the Review - Needed The PR is being reviewed label Apr 3, 2021

Rob Holt (rjmholt) merged commit 8f5ae72 into PowerShell:master Apr 12, 2021

ghost removed the Review - Needed The PR is being reviewed label Apr 12, 2021

Ilya (iSazonov) added this to the 7.2.0-preview.5 milestone Apr 13, 2021

ghost mentioned this pull request Apr 14, 2021

Bug: PowerShell cannot handle a file with long non-ascii file name #13551

Closed

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026

Fix redundant iteration while splitting lines (PowerShell#14851)

dc21d3e

Conversation

Steve (hez2010) commented Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Ilya (iSazonov) commented Feb 19, 2021

Uh oh!

Steve (hez2010) commented Feb 19, 2021

Uh oh!

Deleted user (ghost) commented Feb 27, 2021

Uh oh!

Steve (hez2010) commented Feb 28, 2021

Uh oh!

Ilya (iSazonov) commented Feb 28, 2021

Uh oh!

Steve (hez2010) commented Mar 1, 2021

Uh oh!

Ilya (iSazonov) commented Mar 1, 2021 • edited by unfurl-links Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Steve (hez2010) commented Mar 10, 2021

Uh oh!

Ilya (iSazonov) commented Mar 10, 2021

Uh oh!

azure-pipelines Bot commented Mar 10, 2021

Uh oh!

Ilya (iSazonov) commented Mar 10, 2021

Uh oh!

Steve (hez2010) commented Mar 15, 2021

Uh oh!

xtqqczze Mar 16, 2021

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

Deleted user (ghost) commented Mar 24, 2021

Uh oh!

Steve (hez2010) commented Mar 26, 2021

Uh oh!

Ilya (iSazonov) commented Mar 27, 2021

Uh oh!

Deleted user (ghost) commented Apr 3, 2021

Uh oh!

Steve (hez2010) commented Apr 5, 2021

Uh oh!

Ilya (iSazonov) commented Apr 13, 2021

Uh oh!

Deleted user (ghost) commented Apr 14, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Steve (hez2010) commented Feb 19, 2021 •

edited

Loading

Ilya (iSazonov) commented Mar 1, 2021 •

edited by unfurl-links Bot

Loading