Skip to content

Conversation

@vexx32
Copy link
Collaborator

@vexx32 vexx32 commented Jan 15, 2020

PR Summary

We were not flushing the input buffer immediately when a different type is encountered. This caused some odd behaviour when piping in a mix of bools and ints. Fix is to immediately flush the input buffer when the incoming object is a different type than anything we have buffered.

This PR also contains a commit which cuts down the visual representation of booleans to 1 byte. Marshal.SizeOf() actually returns 4 for boolean values, since .NET pads Boolean values apparently to make them easier to compare with on modern processors which tend to be optimized for 32 or 64 bit values. See this SO question for reference.

I modified some of the logic to skip checking the size if the input value is Boolean, and simply represent it with an 0x1 or 0x0 byte to keep the representation as effective as possible.

PR Context

Resolves #11585

/cc @SteveL-MSFT @iSazonov

PR Checklist

We were not flushing the input buffer immediately when a different
type is encountered.
This caused some odd behaviour when piping in a mix of bools and ints.
Fix is to immediately flush the input buffer when the incoming object is
a different type than anything we have buffered.
@ghost ghost assigned iSazonov Jan 15, 2020
@vexx32 vexx32 added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 15, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Jan 15, 2020
@vexx32 vexx32 force-pushed the FormatHex/FixGrouping branch from a7e7381 to 74b2569 Compare January 15, 2020 12:20
@vexx32 vexx32 force-pushed the FormatHex/FixGrouping branch from 74b2569 to fab4ef2 Compare January 15, 2020 12:44
@vexx32 vexx32 force-pushed the FormatHex/FixGrouping branch from fab4ef2 to 6fa1ead Compare January 15, 2020 13:25
@vexx32 vexx32 force-pushed the FormatHex/FixGrouping branch from 6fa1ead to f39fc4b Compare January 15, 2020 14:40
@vexx32
Copy link
Collaborator Author

vexx32 commented Jan 15, 2020

OK that should do it for the tests this time 'round. Very much starting to feel like we need to rewrite those ones at some point. Anyhow, should be good to go once CI passes @SteveL-MSFT

@vexx32 vexx32 force-pushed the FormatHex/FixGrouping branch from f39fc4b to 904bdeb Compare January 15, 2020 15:06
@iSazonov iSazonov merged commit b71d983 into PowerShell:master Jan 24, 2020
@vexx32 vexx32 deleted the FormatHex/FixGrouping branch January 24, 2020 07:53
@ghost
Copy link

ghost commented Mar 26, 2020

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

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

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatting issue in Format-Hex when Boolean's are involved in a mixed array.

4 participants