Skip to content

Added -Top and -Bottom aliases for -First and -Last#2446

Closed
KirkMunro wants to merge 1 commit intoPowerShell:masterfrom
KirkMunro:select-top-bottom
Closed

Added -Top and -Bottom aliases for -First and -Last#2446
KirkMunro wants to merge 1 commit intoPowerShell:masterfrom
KirkMunro:select-top-bottom

Conversation

@KirkMunro
Copy link
Copy Markdown
Contributor

@KirkMunro KirkMunro commented Oct 8, 2016

If you watch recorded demos of introductory PowerShell sessions delivered by people like @jpsnover, they inevitably show the famous select top 10 processes pipeline. When they do, they invoke something like this (with or without aliases, etc.):

Get-Process | sort WorkingSet64 -desc | select -first 10

As they are entering that line, they very often (almost always) say top 10 as they type "-first 10". It's a small thing, but it's how the mind works. We want the top 10 processes, and technically, in the output, we're getting the top 10 since they are output vertically. I've always felt this would be better if the -First parameter of Select-Object had a -Top alias and the -Last parameter of Select-Object had a -Bottom alias. Such a change would allow for this instead:

gps | sort WorkingSet64 -desc | select -top 10

Now that PowerShell is open source, I decided to add them, see what you think. New parameter aliases are defined and new Pester tests are included for verification.

@msftclas
Copy link
Copy Markdown

msftclas commented Oct 8, 2016

Hi @KirkMunro, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@SteveL-MSFT
Copy link
Copy Markdown
Member

I like the intent, but the problem is that top and bottom implies sorting unlike first and bottom which is not sorted. So ideally you would have:

get-process | select -top 10

but is this top 10 by cpu usage, workingset, etc?

Now we would have to include sorting semantics but this breaks the "do one thing and do it well" principle unless we can determine significant common use cases to justify it. For example would we want to support this?

get-process | select -top 10 -select workingset64 -desc

But that didn't really save much typing nor improve readability.

@lzybkr
Copy link
Copy Markdown
Contributor

lzybkr commented Oct 8, 2016

I agree with @SteveL-MSFT - -Top and -Bottom imply sorting whereas -First and -Last do not.

Now maybe it makes sense to add -Top and -Bottom to Sort-Object. That could even enable a faster sort by avoiding sorting elements that won't be written to the pipe.

@jpsnover
Copy link
Copy Markdown
Contributor

jpsnover commented Oct 8, 2016

I love @lzybkr idea of adding this to sort-object!

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 8, 2016

I'm "inside" this cmdlet last week and can say that it is very intuitive to the user UX and very controversial, it is a trick in the code, and any even slightly change this code can easily break down. In general I think this cmdlet should be replaced with a newer, which meets modern requirements to cmdlets (See #2421 ).

I like the idea of KirkMunro and I agree with @lzybkr that this idea should be implemented in Sort-Object.

@KirkMunro
Copy link
Copy Markdown
Contributor Author

I agree with all of the comments, and I sat on this idea for a while for exactly the reason that -Top and -Bottom might imply sorting. I had considered creating new parameter sets for Select-Object for -Top/-Bottom that included sorting, but I couldn't convince myself that baking sorting into Select-Object was the right solution since Select-Object is a pretty well defined atomic command. In the end I decided that as parameter aliases it wasn't so bad, and that if I was going to go the route of actually doing sorting and selecting I might create a different cmdlet instead. I hadn't thought about putting -Top/-Bottom as parameters on Sort-Object though, and I really like that idea -- it makes much more sense in the context of Sort-Object. Is that something you want to run with @lzybkr or do you mind if I have a look?

@lzybkr
Copy link
Copy Markdown
Contributor

lzybkr commented Oct 9, 2016

I have plenty on my plate, so consider the idea up for grabs.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 9, 2016

@KirkMunro You caught the game - you cook 😊

(I believe it would be appropriate to close this PR and open a new Issue with full description of the idea.)

@KirkMunro
Copy link
Copy Markdown
Contributor Author

@lzybkr Thanks, I'll have a look.

@iSazonov Of course, the proposed sort-object changes will be submitted under a new PR (and I can probably create an accompanying Issue as well).

@KirkMunro KirkMunro closed this Oct 9, 2016
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 9, 2016

https://github.com/PowerShell/PowerShell/blob/master/docs/maintainers/issue-management.md
I think it is a good practice to preliminary create a Issue (as Issue-Enhancement in this time).
Waiting news from you @KirkMunro !

@KirkMunro
Copy link
Copy Markdown
Contributor Author

@iSazonov I agree about creating an issue. I needed to do some research/experimentation first though to see what the solution might look like, see how other parameters would play with this feature, etc. I'm also getting ready for a conference, so busy right now, but I will move this along as soon as possible.

@iSazonov
Copy link
Copy Markdown
Collaborator

@KirkMunro Create Issue now and ask PowershellTeam to assign you. This will eliminate possible conflicts. You and we can also get early feedback to the time when you will have the opportunity to code. Good luck!

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.

6 participants