Skip to content
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

Add AutoSize parameter to New-MDTable function #20

Merged
merged 1 commit into from
Feb 27, 2020
Merged

Add AutoSize parameter to New-MDTable function #20

merged 1 commit into from
Feb 27, 2020

Conversation

al-cheb
Copy link
Contributor

@al-cheb al-cheb commented Feb 24, 2020

AutoSize - Indicates that the function adjusts the column size and number of columns based on the width of the data

PS > gsv d* | New-MDTable -Columns ([ordered]@{Name=$null;DisplayName=$null}) -AutoSize
| Name                                     | DisplayName                                                                        |
| ---------------------------------------- | ---------------------------------------------------------------------------------- |
| DcomLaunch                               | DCOM Server Process Launcher                                                       |
| defragsvc                                | Optimize drives                                                                    |
| DeviceAssociationService                 | Device Association Service                                                         |
| DeviceInstall                            | Device Install Service                                                             |
| DevicePickerUserSvc_4c2ee                | DevicePicker_4c2ee                                                                 |
| DevicesFlowUserSvc_4c2ee                 | DevicesFlow_4c2ee                                                                  |
| DevQueryBroker                           | DevQuery Background Discovery Broker                                               |
| Dhcp                                     | DHCP Client                                                                        |
| diagnosticshub.standardcollector.service | Microsoft (R) Diagnostics Hub Standard Collector Service                           |
| diagsvc                                  | Diagnostic Execution Service                                                       |
| DiagTrack                                | Connected User Experiences and Telemetry                                           |
| DisplayEnhancementService                | Display Enhancement Service                                                        |
| DmEnrollmentSvc                          | Device Management Enrollment Service                                               |
| dmwappushservice                         | Device Management Wireless Application Protocol (WAP) Push message Routing Service |
| Dnscache                                 | DNS Client                                                                         |
| DoSvc                                    | Delivery Optimization                                                              |
| dot3svc                                  | Wired AutoConfig                                                                   |
| DPS                                      | Diagnostic Policy Service                                                          |
| DsmSvc                                   | Device Setup Manager                                                               |
| DsSvc                                    | Data Sharing Service

@Sarafian
Copy link
Owner

@al-cheb before we proceed I need to understand the use case.

If I understand correctly, you want this

| Name | DisplayName |
| ------- | -------------- |
| DcomLaunch | DCOM Server Process Launcher |
| diagnosticshub.standardcollector.service | Microsoft (R) Diagnostics Hub Standard Collector Service |
| dmwappushservice | Device Management Wireless Application Protocol (WAP) Push message Routing Service |

to render like this

| Name                                     | DisplayName                                                                        |
| ---------------------------------------- | ---------------------------------------------------------------------------------- |
| DcomLaunch                               | DCOM Server Process Launcher                                                       |
| diagnosticshub.standardcollector.service | Microsoft (R) Diagnostics Hub Standard Collector Service                           |
| dmwappushservice                         | Device Management Wireless Application Protocol (WAP) Push message Routing Service 

Set aside that there is a trailing | missing, I wonder why do you need this? I've considered that I'm lacking something in markdown and the rendering would be somehow different but I checked with an online renderer and I don't see the difference.

For example the original

Name DisplayName
DcomLaunch DCOM Server Process Launcher
diagnosticshub.standardcollector.service Microsoft (R) Diagnostics Hub Standard Collector Service
dmwappushservice Device Management Wireless Application Protocol (WAP) Push message Routing Service

And what you propose

Name DisplayName
DcomLaunch DCOM Server Process Launcher
diagnosticshub.standardcollector.service Microsoft (R) Diagnostics Hub Standard Collector Service
dmwappushservice Device Management Wireless Application Protocol (WAP) Push message Routing Service

I don't see the difference to be honest

@al-cheb
Copy link
Contributor Author

al-cheb commented Feb 24, 2020

@Sarafian
This is just a nitpick to format columns by maximum length of the max column element instead of max elements for whole collections. I totally agree with you that render looks the same, but the end raw file with very big values looks a little bit strange.

With AutoSize Raw file:
"" | Select name,@{n="id";e={"t"*1000}},command | New-MDTable -AutoSize | out-string -width 2000 > file.txt

Without AutoSize Raw file:
"" | Select name,@{n="id";e={"t"*1000}},command | New-MDTable | out-string -width 2000 > file.txt

@Sarafian
Copy link
Owner

Sarafian commented Feb 24, 2020

Now I understand and you've actually given me some food for thought. I've always approached this module's output from the perspective of manually writing the same and I would never align my comment as described because then I would always have to manage the spaces. What if a column got bigger or smaller, do I need to adjust every other line? But, this is generated markdown and from that perspective, it does make sense to produce evenly aligned markdown because the invention is not to edit but to generate and overwrite again.

I don't like the AutoSize as parameter name and that requires some thinking. I also need to check the pipeline variant because you need to work on the contents of the entire array. I think I didn't see the unit tests as well. Could be wrong because I did a quick check earlier today.

The idea had merit but some work is needed for now. If you have other ideas for the parameter name, more in the direction of "beauty" or "alignment" then please share.

@Sarafian
Copy link
Owner

Got some time to work on this. I figured out that we were both incorrect. The default mode of the module is to take the maximum length of any given cell and use it for all including the headers. Your proposal with -AutoSize is to do the same but per column and I decided to consider your proposal as a bug. There is no point in doing the default bloated output. Instead your proposal with -AutoSize becomes default behavior and I'll add a -Shrink to do what I thought was happening, that is shrink each cell to the required length.

@al-cheb
Copy link
Contributor Author

al-cheb commented Feb 27, 2020

@Sarafian, Thank you. Looks good to me.

Sarafian pushed a commit that referenced this pull request Feb 27, 2020
@Sarafian Sarafian merged commit ddc02a2 into Sarafian:master Feb 27, 2020
@Sarafian
Copy link
Owner

Changes available in version 1.9. Sorry for skipping some but I had some issues with obsolete publishing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants