-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
Issues with AboutEnumeration #372
Comments
Yep, that's... wow, that looks like a real pain to find. Well done, and thankyou! I'll get that one fixed.
Yeah, I see the logic error there. That should be a string, not a scriptblock, and convert just before it's used.
Hmm... I don't think so. The overload definitions for
The overload you're suggesting we use there is a generic method, and that call is very fragile -- if a variable containing a value of the wrong type is used as the Specifying the type with the other overload is a better bet, I think; it's too easy to have it fail otherwise. It's not incorrect, by any means, it's just a different overload for the method. 🙂 |
Using a raw scriptblock will break the parser in earlier versions. Fix is to define it as a string and parse that just before execution.
Hmm, looks like the additional overloads are indeed missing in Windows PowerShell. I wasn't aware they were actually quite new. I'll have a look at what we can do there... |
@indented-automation do you have any suggestions on that last point? Code is here: PSKoans/PSKoans/Koans/Constructs and Patterns/AboutEnumerations.Koans.ps1 Lines 261 to 290 in 1a0fb29
The two overloads that take a specific |
I agree that the untyped TryParse is fragile, I suppose that I should have mentioned that I had gone looking for the additional overload definitions, and was unable to locate them. I suppose this is the excitement of working through these things, when the environment is so highly dynamic. Looking forward to seeing what @indented-automation has to say. |
PS is bit to free with how it binds arguments. Was sure I'd used that method before in Win PS... apparently not. It can be swapped to the more specific Chris |
That's one option, but I don't think that TryParse method is any more specific... I had a quick look at that, but unless I missed something, it looks like it just inherits the same fragile methods from Enum itself. That said, provided the variable you're passing in as reference is of the right Enum type, it'll still work okay. |
* ♻️ Cleanup old test file Refactor slightly, match file more to current style. Rename a few tests to make more sense. * ✅ Add regression test We can test file ASTs during our static analysis checks. Added a test to check we don't have any nested It blocks in the file. * 🐛 (#372) Fix missing / extra brace * 📝 🎨 Adjust AboutEnumeration formatting * 🐛 (#372) Delay parsing of enum Using a raw scriptblock will break the parser in earlier versions. Fix is to define it as a string and parse that just before execution.
That part of the example was fine, I obviously only tested it properly on PS Core which is why we're here :) $DayOfWeek = [DayOfWeek]::Sunday
[DayOfWeek]::TryParse('Wednesday', [Ref]$DayOfWeek) This at least works in Win PS compared with the original :-D |
Might be worth an added note that you have to pass in a variable that already contains one of the enum values for it to work as well, but yep! 😄 |
First issue
Issue #369, still seems to be broken, I tried the Update-Koans, I even copied directly from the Git Repository, still broken. So to fix "You are already in a test case." issue
At line 112 add a "}"
At line 628 remove a "}"
That was lots of fun to find! (head shaking)
Second Issue
The following code, found at line 228, is not going to compile unless you are using Powershell Core 6.2.0 or better, and the Skip command in the test doesn't keep it from compiling
enum Int64Enum : Int64 { LargeValue = 9223370000000000000 }
I couldn't get it to work with Powershell Core 7.0 either, but I didn't try very hard.
I simply removed the test "It 'can represent other integer in PowerShell Core'" from the Koan and kept on rolling.
It is a cool concept that you are introducing but unless you are going to force using Powershell Core 6.2.0 or better, it may not be worth hanging on to.
Third Issue
At line 286, the [Enum]::TryParse is being used incorrectly, the $enumType is not needed
The code should be corrected to
$result = [Enum]::TryParse( $valueToParse, [Ref]$parseResult)
The text was updated successfully, but these errors were encountered: