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

Range operator (aka DotDot operator) with char operands in pipeline throws error #5519

Closed
TimCurwick opened this Issue Nov 21, 2017 · 8 comments

Comments

Projects
None yet
7 participants
@TimCurwick
Contributor

TimCurwick commented Nov 21, 2017

Steps to reproduce

A'..'B'
'A'..'B' | ForEach-Object { $_ }

Expected behavior

A
B

Actual behavior

Cannot convert value "A" to type "System.Int32". Error: "Input string was not in a correct format."
At line:1 char:1

  • 'A'..'B' | ForEach-Object { $_ }
  • CategoryInfo : InvalidArgument: (:) [], RuntimeException
  • FullyQualifiedErrorId : InvalidCastFromStringToInteger```

Environment data

----                           -----
PSVersion                      6.0.0-rc
PSEdition                      Core
GitCommitId                    v6.0.0-rc
OS                             Microsoft Windows 10.0.15063
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0```
@markekraus

This comment has been minimized.

Show comment
Hide comment
@markekraus

markekraus Nov 21, 2017

Collaborator

repros for me.

Collaborator

markekraus commented Nov 21, 2017

repros for me.

@mklement0

This comment has been minimized.

Show comment
Hide comment
@mklement0

mklement0 Nov 21, 2017

Contributor

Interesting; I didn't even know support for character ranges was implemented. Can I suggest changing "dotdot operator" to "range operator" in the title?

Contributor

mklement0 commented Nov 21, 2017

Interesting; I didn't even know support for character ranges was implemented. Can I suggest changing "dotdot operator" to "range operator" in the title?

@TimCurwick

This comment has been minimized.

Show comment
Hide comment
@TimCurwick

TimCurwick Nov 21, 2017

Contributor

I went with "dotdot operator" because that is what was used in the PR that added the character range implementation. #5026

Contributor

TimCurwick commented Nov 21, 2017

I went with "dotdot operator" because that is what was used in the PR that added the character range implementation. #5026

@mklement0

This comment has been minimized.

Show comment
Hide comment
@mklement0

mklement0 Nov 21, 2017

Contributor

I see - that very nonstandard name is the reason that I wasn't able to find this new feature, despite then already knowing that it exists (based on this issue - that someone else would call it "DotDot operator" too hadn't occurred me), so thank you for adding the link.

I guess my mentioning the construct's official name here - range operator - now makes this issue more discoverable and, by extension, now also the linked PR, but my preference is always to have the important keywords in the title, as an in:title search then allows for a more focused search.

Contributor

mklement0 commented Nov 21, 2017

I see - that very nonstandard name is the reason that I wasn't able to find this new feature, despite then already knowing that it exists (based on this issue - that someone else would call it "DotDot operator" too hadn't occurred me), so thank you for adding the link.

I guess my mentioning the construct's official name here - range operator - now makes this issue more discoverable and, by extension, now also the linked PR, but my preference is always to have the important keywords in the title, as an in:title search then allows for a more focused search.

@mklement0

This comment has been minimized.

Show comment
Hide comment
@mklement0

mklement0 Nov 21, 2017

Contributor

Another datapoint (and workaround): Forcing generation of the entire array up front avoids the problem:

('A'..'B') | ForEach-Object { $_ }  # OK, due to (...)
Contributor

mklement0 commented Nov 21, 2017

Another datapoint (and workaround): Forcing generation of the entire array up front avoids the problem:

('A'..'B') | ForEach-Object { $_ }  # OK, due to (...)
@IISResetMe

This comment has been minimized.

Show comment
Hide comment
@IISResetMe

IISResetMe Nov 21, 2017

Contributor

The output from Trace-Command strongly suggests that this code path is reached before the operator is explicitly invoked when the .. appears as the very first operation in a pipeline:

PS C:\> Trace-Command -Expression {'A'..'B' |ForEach-Object { $_ }} -Name * -Option All -FilePath ($t = [System.IO.Path]::GetTempFileName()) 2>$null; Get-Content $t -TotalCount 2
TypeConversion Information: 0 : Converting to integer.
TypeConversion Information: 0 : Exception converting to integer: "Input string was not in a correct format.".

Why, I'm not entirely certain yet, will keep digging. While this is certainly not the intended behavior of the new [char] overload for .., it doesn't seem to break any previously defined behavior (this is basically the expected behavior for $string..$otherstring prior to #5026)

Re: the "nonstandard" name, I took it from the token that represents the underlying range operator :-)

Contributor

IISResetMe commented Nov 21, 2017

The output from Trace-Command strongly suggests that this code path is reached before the operator is explicitly invoked when the .. appears as the very first operation in a pipeline:

PS C:\> Trace-Command -Expression {'A'..'B' |ForEach-Object { $_ }} -Name * -Option All -FilePath ($t = [System.IO.Path]::GetTempFileName()) 2>$null; Get-Content $t -TotalCount 2
TypeConversion Information: 0 : Converting to integer.
TypeConversion Information: 0 : Exception converting to integer: "Input string was not in a correct format.".

Why, I'm not entirely certain yet, will keep digging. While this is certainly not the intended behavior of the new [char] overload for .., it doesn't seem to break any previously defined behavior (this is basically the expected behavior for $string..$otherstring prior to #5026)

Re: the "nonstandard" name, I took it from the token that represents the underlying range operator :-)

@SteveL-MSFT SteveL-MSFT changed the title from DotDot operator with char operands in pipeline throws error to Range operator (aka DotDot operator) with char operands in pipeline throws error Nov 21, 2017

@mklement0

This comment has been minimized.

Show comment
Hide comment
@mklement0

mklement0 Nov 22, 2017

Contributor

@SteveL-MSFT: Your title edit just made my day, thanks (I wish I were kidding).

@IISResetMe: Fair enough, and perhaps "DotDot operator" is reasonably well-known; my term for it came from Get-Help about_Operators, and I would argue that it's the more sensible name:

Operators should be named for their purpose (only), not their syntactic form (too).

I won't repeat my rant about about the "dot sourcing operator" here.
Although, who's to say? Perhaps it's time for the "asterisk multiplication operator", or the "hyphen-minus subtraction operator", and, last but not least, the "dot-dot range operator".

Contributor

mklement0 commented Nov 22, 2017

@SteveL-MSFT: Your title edit just made my day, thanks (I wish I were kidding).

@IISResetMe: Fair enough, and perhaps "DotDot operator" is reasonably well-known; my term for it came from Get-Help about_Operators, and I would argue that it's the more sensible name:

Operators should be named for their purpose (only), not their syntactic form (too).

I won't repeat my rant about about the "dot sourcing operator" here.
Although, who's to say? Perhaps it's time for the "asterisk multiplication operator", or the "hyphen-minus subtraction operator", and, last but not least, the "dot-dot range operator".

@lzybkr

This comment has been minimized.

Show comment
Hide comment
@lzybkr

lzybkr Nov 22, 2017

Member

@IISResetMe - the idea was to name tokens after the syntactic form because it might not make sense if a token was allowed in some new context, e.g. if .. was allowed where it is no longer the range operator.

Member

lzybkr commented Nov 22, 2017

@IISResetMe - the idea was to name tokens after the syntactic form because it might not make sense if a token was allowed in some new context, e.g. if .. was allowed where it is no longer the range operator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment