PowerShell / PowerShell-RFC

RFC (Request for Comments) documents for community feedback on design changes and improvements to PowerShell ecosystem
MIT License
423 stars 121 forks source link

RFC0007-Weak-Aliases comments #16

Closed lzybkr closed 7 years ago

lzybkr commented 7 years ago

[PowerShell Committee has Rejected this RFC, see decision for details]

This issue is to discuss the weak aliases RFC.

This proposal helps address issues related to the curl/wget aliases introduced in PowerShell V3.

The discussion should focus on the strengths and weaknesses of this specific proposal and on avoiding or minimizing breaking changes. Ideally, existing scripts should continue to run without any changes when a new version of Windows PowerShell is installed.

Note that doing nothing is the only way to completely avoid breaking changes - the goal of this RFC is to find a compromise.

This RFC does propose a breaking change, but only in scripts where curl and wget are meant to call Invoke-WebRequest but curl.exe or wget.exe are found via the path. We don't have data on how often this might be an issue - but if a future version of Windows installs curl.exe and wget.exe by default, then it may be pointless to implement this RFC as that would be equivalent to removing the aliases.

kuldeepdhaka commented 7 years ago

@jpsnover Please join the discussion. I dont think, things are going as the community was expecting.

KevinMarquette commented 7 years ago

Regarding the RFC analysis, how many of those scripts would work on Linux? We are already breaking them on that platform by removing those aliases.

kuldeepdhaka commented 7 years ago

Im really concerned, how the issue has been mishandled.

Community is expecting that on the broad level, all the incorrect alias will be removed. now, the RFC is playing a trick that: just fix the high profile "curl"/"wget" case and continue the embrace, extend, extinguish

dlwyatt commented 7 years ago

The ones that were clearly identified as executing Invoke-WebRequest would obviously break. The 400+ or so where it says "curl wasn't used as a command", who knows.

However, since there are no existing customers relying on those scripts to run on the non-existent prior versions of PowerShell on Linux, you can hardly call that a breaking change.

dlwyatt commented 7 years ago

@kuldeepdhaka We get it. You've said that phrase half a dozen times in this discussion alone. Get some new material.

kuldeepdhaka commented 7 years ago

We get it. You've said that phrase half a dozen times in this discussion alone. Get some new material.

Where is the Acknowledgement for my statement? What we need is a clear cut anwser, not just lame excuses.

alejandro5042 commented 7 years ago

@riverar Actually, flagging aliases was a temporary bug in Visual Studio Code that was fixed in yesterday's update. See: https://github.com/PowerShell/vscode-powershell/issues/246

You're already going to have to learn an entirely new approach to using the command-line to use PowerShell, so I just don't know that gci instead of dir or sl instead of cd is really going to be make or break.

Exactly, so there's no harm in keeping them. In PowerShell land, ls acts differently--and I expect it to. I expect that the majority of commands take dashed arguments and return rich objects. Every language has a file API with similar terminology--nobody has a monopoly on terms.

However, I do see a need to resolve this. I had an issue with a highlight alias I wrote for a Write-Host wrapper with regex highlighting--it conflicted with the popular highlight.exe.

Although seemingly helpful, I doubt weak aliases will satisfy. I'd rather have an operator that distinguishes between which version I wish to call (e.g. ^curl to escape to the parent shell), or a built-in module (scope) called native or system that always resolve to native commands:

native\curl    # calls curl.exe
system\curl    # alternative, but may conflict with the .NET System namespace

Either solution (^curl or native\curl) don't break existing users, helps you guys, and solves my highlight issue without resorting to a weak alias, which is really a weighting problem.

dlwyatt commented 7 years ago

@kuldeepdhaka Acknowledgement of what, exactly? Your conspiracy theory bullshit about embrace / extend / extinguish? This discussion is supposed to be about the merits of the RFC, which I also think is not a good idea. However, I don't make accusations about the thoughts or motivations of the people involved.

If you want to be part of the solution, try to find a way to make this right while not causing too much pain for existing PowerShell users on Windows. Otherwise, piss off and let other people discuss it without your noise.

DrPizza commented 7 years ago

Exactly, so there's no harm in keeping them. In PowerShell land, ls acts differently--and I expect it to. I expect that the majority of commands take dashed arguments and return rich objects. Every language has a file API with similar terminology--nobody has a monopoly on terms.

For simple commands like ls or cd that may be tolerable (though I hate it), but for something as rich and complex as curl or wget? PowerShell needs to get the hell out of the way.

KevinMarquette commented 7 years ago

Outside of fixing this issue, are there any other ways we can get value out of weak aliases? Or other issues it may cause the community?

kuldeepdhaka commented 7 years ago

piss off

@dlwyatt All aside, please maintain professionalism.

dlwyatt commented 7 years ago

While a step forward, I don't think weak aliases will help. I'd rather have an operator that distinguishes between which version I wish to call (e.g. ^curl to escape to the parent shell), or a built-in module (scope) called native or system that always resolve to native commands:

I was thinking of something similar, but using a PowerShell provider. Executing App:curl would essentially be a wrapper for & (Get-Command curl -CommandType Application), that sort of thing.

However, when you get down to it, the pushback from the Linux community isn't going to stop until you can just type curl and have it run the executable, regardless of platform. The trick is figuring out how best to get from here to there.

dlwyatt commented 7 years ago

@dlwyatt All aside, please maintain professionalism.

That's rich, considering the source. Read any of your own comments recently?

kuldeepdhaka commented 7 years ago

@dlwyatt I projected my view on what is going on/think. Using "piss off" is UN-professionalism. (atleast to me) i didn't use any Vulgar/Slangs words.

dlwyatt commented 7 years ago

What you're doing is trolling. What I'm doing is giving pretty much my minimum force response to a troll.

PS: You missed that I also said "bullshit".

KevinMarquette commented 7 years ago

What if we made them deprecated aliases instead.

New-Alias -Name Process -Value Get-Process -Deprecated

It would work just like the weak alias logic but would add a warning message when it was used. If the command exists either in the path or as a new alias, then the strong one would be called and there would be no message.

alejandro5042 commented 7 years ago

However, when you get down to it, the pushback from the Linux community isn't going to stop until you can just type curl and have it run the executable, regardless of platform. The trick is figuring out how best to get from here to there.

Hmm, agreed. Perhaps we can iterate on the module idea.

I just tested that that ambiguity exists between two functions with the same name exported from two auto-loaded modules, which is unfortunate. Worse, Get-Command only shows one of them. You can disambiguate by prefixing with the module name and it calls the one you want. You can also pass -Module <name> and Get-Command will return you want.

The system PATH has a way to disambiguate: an ordering of the folders listed in the variable. And within a folder, you cannot have a file with the same name. Perhaps we can do an order in the way modules are inspected for functions. That way, when you type curl, you get native\curl.

I'm brainstorming here, and I realize that this does not help me because I want to type ls and get Get-ChildItem. But perhaps this can help point the conversation in a better direction.

dlwyatt commented 7 years ago

Personally, I'm of the opinion that sooner or later we're going to have to break some eggs to make the omelette. Microsoft does try to minimize this, but new major releases of PowerShell have always broken something. (Here's looking at you, Exchange!)

Some mistakes have been made in the first decade or so of PowerShell's development, and I'd rather see a firm decision of "let's fix it properly" or "let's live with it" rather than adding more confusion through a feature like weak aliases. As long as the community gets a reasonable lead time warning of the upcoming changes (and preferably a good way to future-proof their scripts ahead of the change; in this case, just updating their calls to use Invoke-WebRequest or iwr), it should be okay.

There may be those users who are running old versions of scripts / modules and don't know or care how to update them or modify them, but there's no way to help that. At least many of us will get some extra work troubleshooting and fixing broken scripts for a while. :)

riverar commented 7 years ago

@alejandro5042 Thanks for the heads up, but the underlying Microsoft PSScriptAnalyzer still warns about alias use. Visual Studio Code just continues to hide them, for some unknown reason. I only bring this up to note that someone in the PowerShell team has already had this discussion internally and the consensus was that aliases are bad.

KevinMarquette commented 7 years ago

@riverar it has been a best practice for a long time not to use them in scripts that are shared. The more verbose versions makes the code easier to read and learn from for those new to Powershell. The community is in support of that one.

kuldeepdhaka commented 7 years ago

@dlwyatt Better is to just remove the unix alias completly in next major version. I agree with you that introducing new confusing feature isnt going to help in long run.

tmintner commented 7 years ago

I'm in favor of removing all aliases that conflict with binaries. I also think it would be a great idea as mentioned by @KevinMarquette to include a module for all of the legacy aliases. I like those options much better than any of the workaround proposed by the RFC or on this thread.

dragonwolf83 commented 7 years ago

While a step forward, I don't think weak aliases will help. I'd rather have an operator that distinguishes between which version I wish to call (e.g. ^curl to escape to the parent shell), or a built-in module (scope) called native or system that always resolve to native commands:

Taking your idea a step further, a script block would be a great way to do this. This is consistent with usage from Invoke-Command and InlineScript whose behavior closely matches this logic. See below:

ShellScript { curl -x -y }
^{ curl -x -y }
^curl -x -y

ShellScript could be the full command with ^ being the alias. Using a script block would allow running several native commands without having to alias each one.

However, none of these address Microsoft's concern around breaking change. These suggestions require changes to scripts. I get it, I deal with this on a daily basis (I'm the one nagging devs in my job to fix breaking changes that affect 30,000+ reports).

The best proposals I've seen so far take this out of the scripting level into the server level and engine level. Either of the following would work:

As a sysadmin, I should be able to control the behavior for all my scripts on all servers., If it breaks, I can fix it easily.

There is still major disagreement though on what aliases should be removed. Curl and wget are pretty much agreed on moving, but the others are much more debatable if they should remain or not as defaults.

lzybkr commented 7 years ago

@KevinMarquette - PowerShell already has the notion of a weak alias. If you try to run ChildItem, there is no such command, so PowerShell will try Get-ChildItem. This works for any command that is not found.

If you try Get-Command ChildItem, nothing is returned, so why ChildItem works is a mystery to folks that discover it.

Introducing a weak alias as a formal concept means that Get-Command ChildItem could return an AliasInfo object with the Weak property set to true.

KevinMarquette commented 7 years ago

@lzybkr Thanks for that extra info. I can't say I have ever thought to run Get-Command for one of those before. Interestingly enough, Get-Help ChildItem does find the correct help.

With that said, can we replace the curl alias with get-curl and call this one resolved. It looks like it does what the RFC is trying to do with the formal weak aliases. (although get-wget looks like an ugly hack and would make it worth implementing it correctly )

GeeLaw commented 7 years ago

In reply to @dlwyatt

I'm not sure what you mean by a "binary pipe" here, or why that would affect command precedence, but changing command precedence in this way would be such a massive breaking change that it will never happen. (Consider all the discussion that had to go into just removing two fairly little-used aliases, by comparison.)

By binary pipe I mean a feature with which one can get the binary stream in/out of a native utility (instead of string, as PowerShell is not good at guessing the encoding of a stream, and even, some utilities does not output a text stream). Since PowerShell cmdlets/functions does NOT give/accept binary streams (they accept objects) and native utilities does NOT give/accept objects (they accept binary streams), it is pretty easy to understand what the user means when using a "binary pipe".

The binary pipe has not been a PowerShell feature therefore nothing is broken by introducing such rules with binary pipes. You cannot break nonexistent things.

The only flaw in this design is that, when a PowerShell cmdlet/function accepts a Stream object, or a byte[], it'll be hard to guess.

torgro commented 7 years ago

There are multiple things to consider here:

Linux/OSX This is the Alpha release of Powershell on this platform.

Windows On gen 5.1 of powershell. May use wget and/or curl either as native commands or as an alias in Powershell

Since I started using Powershell (many, many years ago) using aliases in your script has ALWAYS been against best practice guides from Microsoft. At the gates of Mordor, there are piles of good intentions and best practices as some people just ignores them or only follows them to a certain degree. Thus we are left in a pickle with a riddle.

Current Powershell users with version 3 or higher, are facing and handling this issue as we speak. Removing the aliases may be a breaking change for this platform because they did not follow the best practice guides. That is just putting it in binary terms. Please understand that the Powershell team must account for other factors in this.

Introducing Powershell on Linux with the aliases would be a breaking change for them, since they use those aliases for native commands.

Options

If the ultimate goal is to create a scenario where any user on any platform can take any Powershell script/module/function and run it without issues, there is really only one solution. Find a way to gracefully remove the aliases. Thinking about it, introducing weak aliases where the goal is to completely remove them in the future may be a way to go.

Even though you have done a great analysis of the usage of aliases on github and other sources, there are literally billions of Powershell codelines floating on internal file-servers and computers out there. Maybe we should create some helper functions people could use to analyze their usage of aliases like the Gist in the RFC, or just get people to use the PSScriptAnalyzer module.

atanasa commented 7 years ago

Have you considered the option of removing the aliases in the *nix version while keeping them in the Windows version? Scripts are not portable out of the box so any issues with the aliases would be handled as part of the porting effort.

kuldeepdhaka commented 7 years ago

@atanasa That would only cause confusion to everybody.

lzybkr commented 7 years ago

@KevinMarquette - yeah, renaming the aliases to get-curl and get-wget would likely have the intended effect, but I haven't tested. But I have a feeling that change itself would break a few scripts that use those command names to install curl or wget.

@atanasa - that's how it is right now - the aliases curl and wget do not exist in PowerShell Core, this applies to Linux, Mac, and Windows. The aliases only exist in Windows PowerShell.

GeeLaw commented 7 years ago

@atanasa curl and wget as aliases aren't really available for PS for *nix or PSCore. These aliases are only there for Windows PowerShell.

I think the whole thing is just a porting issue and people really should leave these alises available for Windows PowerShell by default (for backward compatibility reasons). Anyone who ever wants to write portable PowerShell scripts should have never used and will never use aliases, even the ones like iwr.

GeeLaw commented 7 years ago

@torgro I agree with most of what you said but I just want to politely point out that

Introducing Powershell on Linux with the aliases would be a breaking change for them, since they use those aliases for native commands.

is not logically correct. Though introducing them is troublesome, and I agree that we should avoid them in PowerShell Core / for nix, it is surely not a breaking change since there wasn't PowerShell on nix before. When you say a "breaking change", it means something that used to work no longer works now. But since PowerShell was never on nix before, cURL did not work on PowerShell for nix. In simple words, it should not be taken for granted that ABC should work on PowerShell for *nix just because PowerShell is supposed to support native commands.

kilasuit commented 7 years ago

The problem with removing the curl alias or applying the suggested Weak attribute is that if you take the last example in the help for Invoke-WebRequest and run this on a system the has had curl installed like @dlwyatt mentioned then the example

Invoke-WebRequest -Uri "http://msdn.microsoft.com/en-us/library/aa973757(v=vs.85).aspx"

breaks if you change this to curl after removing the alias like so

Remove-Item Alias:\curl
curl -Uri "http://msdn.microsoft.com/en-us/library/aa973757(v=vs.85).aspx"

as it doesn't output in the same way.

We in the PowerShell Community have long taught that the golden 3 rules for discoverability are always

Get-Command Get-Help Get-Member

and alongside that its also widely taught that Aliases are not for use in scripts - yet we still see people that use them and defend using them in their scripts.

I've also seen organisations roll out to all their servers a $PROFILE.AllUsersAllHosts that removes all aliases

I'm afraid that anyway we go will be a breaking change however that being said I think that we need to do a combination of things to resolve this.

torgro commented 7 years ago

@GeeLaw I politely claim that you are nitpicking. I stand corrected and can live with that, just need more popcorn.

I think the graceful combination of items that @kilasuit has introduced is awesome! Just let me be crystal clear on this, since my logic has abandoned me today, this list:

Be extremely subtle, even to the point of formlessness. Be extremely mysterious, even to the point of soundlessness. Thereby you can be the director of the opponent's fate.

-Sun Tzu

SteveL-MSFT commented 7 years ago

Thanks for all the feedback and suggestions thus far. I'd ask that we continue to try to keep this civil and professional (discuss the RFC/issue, not each other).

The PowerShell Team will meet on Monday to review the feedback and will propose next steps on this RFC . Thanks!

riverar commented 7 years ago

@torgro Minor nit, PSScriptAnalyzer already flags alias use.

kilasuit commented 7 years ago

@riverar - Indeed it does but the PSAvoidUsingCmdletAliases rule outputs only as a Warning Severity - I'm suggesting that for curl & wget this should output an Error instead of warning and be included in a PSCore compatibility ruleset.

HumanEquivalentUnit commented 7 years ago

I think the weak-alias approach is a bad idea. DWIM (Do What I Mean) guesses are great when they guess correctly, but frustrating and confusing when they guess wrongly. This can be a useful tradeoff in situations where they can guess correctly a lot of the time and being correct brings a large benefit - but the weak-alias heuristic does neither. It will often be wrong, it will be wrong in surprising and unpredictable ways (moving a script between servers with/without curl.exe), and the assistance it offers is tiny - a reduction of ~1 line of remove-alias code for annoyed people, who will still be annoyed because this RFC doesn't address what they are actually annoyed about.

The desire to get rid of these aliases is not technical, it's about community expectations and fairness of name reuse; this RFC to keep the curl/wget aliases in PowerShell forever doesn't achieve the goals of the request - 'Linux' users who open PowerShell on a fresh Windows server will get different curl/wget behaviour than they were expecting.

Conclusion: Not this RFC approach. This RFC won't significantly resolve the problem technically or communally, it will leave the annoyed people annoyed with curl/wget aliases still existing, and it will annoy new people with long-term variable trip-you-up behavior. Just "making some change to the aliases so we can say we did" will not stop the complainers from complaining, nor will it stop the confused from being confused.

Desired data: How many of the people complaining would see any compromise change that leaves curl/wget aliases in PS indefinitely, accept it as a genuine effort, and be satisfied with the compromise and pleased with Microsoft, and then be happy?

alejandro5042 commented 7 years ago

I do want to see PowerShell adopted in the Linux/Mac community so building goodwill is important. While I personally don't mind removing curl and wget, I'd be curious what people think of the 17 others shipped with PS, and somewhat relatedly, the 3 from the popular pscx module. Some of these are ps, rm and sort... common commands.

Naturally I did this via PowerShell ;) ...approximating on Windows using the Git bin directory:

PS> ls 'C:\Program Files\Git\usr\bin' -Exclude '`[.exe' | foreach basename | Get-Alias -EA 0 | sort Module, Name | fw DisplayName -GroupBy ModuleName -Column 4

   ModuleName:

cat -> Get-Content             clear -> Clear-Host           cp -> Copy-Item               curl -> Invoke-WebRequest
diff -> Compare-Object         dir -> Get-ChildItem          echo -> Write-Output          kill -> Stop-Process
ls -> Get-ChildItem            mount -> New-PSDrive          mv -> Move-Item               ps -> Get-Process
pwd -> Get-Location            rm -> Remove-Item             rmdir -> Remove-Item          sleep -> Start-Sleep
sort -> Sort-Object            start -> Start-Process        tee -> Tee-Object

   ModuleName: Pscx

ln -> New-Hardlink             tail -> Get-FileTail          touch -> Set-FileTime
GeeLaw commented 7 years ago

@torgro Anyway you like, nitpicking or being rigorous. List item 1, 2 and 4 are great and as already pointed out, 4 has always been there.

Only, I don't think item 3 will work as expected. I do not expect someone (a user of a script) who cannot fix such an issue (identifying curl/wget aliases and correct them) to watch event log. This might do for some corner case admins? cc @kilasuit

torgro commented 7 years ago

@GeeLaw I said I stood corrected, and I did. Just leave at it that, I will :-)

So the scenario is; A script consumer who only uses a script, does not understand it, are unable to fix it and cannot read the eventlog or does not know where to find the eventlog. For those there are no hope. They are the lost souls that rely upon a poorly implemented solution that no one (!!!) can fix. If we can save only one of those poor souls, that actually catches that event in the eventlog, I would say implement it. With the best of me, I can see no harm in it.

@riverar I am fully aware of usage of aliases triggers an warning in PSscriptAnalyzer, why else would I suggest using it? I also second the feature request in PSscriptAnalyzer by @kilasuit to change this warning to an error.

On a side-note; Very few people actually use the module to analyze their scripts in my experience. We can only hope that this will change going forward, however I feel that this rule will have marginal impact on the graceful removal of the aliases. Thus this is something the team will have to take into account in their meeting on Monday. I can only share my thoughts about this, they have to navigate a solution in the RFC. May the force be with you :-)

rkeithhill commented 7 years ago

The problem I have with the RFC proposal is that it allows only the alias creator to tweak where the alias shows up in command resolution order. But in order to write scripts and modules that run correctly "out in the wild", it is really the script author that needs that control IMO.

Today we have the ability to module name qualify a command and I do that a lot in scripts meant to be run on lots of unknown target machines. Instead of Expand-Archive, I'll use Microsoft.PowerShell.Archive\Expand-Archive to make sure I'm getting the command I want because I have no flippin' idea what modules or proxy commands a user has configured on their system that may override a built-in command.

That's why I like the approaches that @dlwyatt and @kilasuit have suggested. Give the script author (and CLI user) a way to bypass the built-in precedence rules to indicate that they don't want an alias or function or script named curl. They want the application named curl instead eg: app:curl or native\curl.

I would like to see the transitional/comfort aliases for curl/wget stay as they are in the "Windows" version of PowerShell to avoid breaking long time users (no matter how you feel about their script authoring practices). I also like the idea of introducing an -Obsolete parameter on New-Alias so these long time "Windows PowerShell" users are notified that the aliases will eventually be removed (perhaps with an easy mechanism to re-enable them).

For the PowerShell Core version, leave in the "PowerShelly" aliases like iwr, gci, gc, etc but leave out all the "transitional/comfort" aliases by default. Create a command to allow a user to easily enable a predefined set of aliases just like how in PSReadLine I can select an edit mode (Windows, VI, Emacs). This command would create the flavor of aliases you want (Windows, Linux, Both). And of course, you could skip this and define the aliases you want in your profile.

FWIW I think PowerShell Core is the time (maybe the last) to take a mulligan and fix some nagging issues even if it introduces some breaking changes (only to PowerShell Core though). I'm not saying break everything but I'm sure there are some nasties that if you had the chance, you'd like to fix. In fact, I think there will be an expectation that existing scripts will require porting to get to run on PowerShell Core (and .NET Core BTW). And if you're going to take that hit for breaking changes anyway ... :-)

riverar commented 7 years ago

@torgro Your words, not mine, were: "Add a new PSScriptAnalyzer rule for these aliases". If you knew they existed, I don't know why you would use the word add. Was just a friendly nit, no worries!

torgro commented 7 years ago

@riverar No worries. I feel like a politician where everyone twists my words :-)

If you are reading this, please use PSScriptAnalyzer! It is good for you and currently it will notify you with a warning if you use any aliases in your code. @kilasuit have suggested (and I have supported this feature) raising the warning to an error level if you use curl or wget in you code.

joeyaiello commented 7 years ago

Really appreciate the discussion here, everyone. I think the conversation has been extremely constructive, but as @SteveL-MSFT said, please remember to be civil.

Also keep in mind when reading this that these are my personal opinions as a member of the PowerShell Committee. As part of becoming an open source project, I believe that we should have opinions which aren't completely reconciled internally at Microsoft, as would be the case with any open-source project that doesn't have internal emails and meetings before going public with a decision. :)

Okay, with that out of the way...After reading all the replies, I think the heart of the discussion comes down to a few questions:

Specific to this RFC:

  1. Is the segment of users who have written PowerShell scripts against the curl alias to Invoke-WebRequest and who also have curl.exe installed on their Windows machines sufficiently small that a compromise approach like the weak aliases would have a minor breaking impact?
  2. Do weak aliases introduce a hidden level of complexity that will make it harder to debug scripts on Windows PowerShell for users who install curl.exe (whether's it's from installing native curl, Git Bash, or by putting Cygwin binaries in your PATH)?

One thing to keep in mind is that PowerShell has a significant of users who operate in the kind of enterprise where they aren't or can't post scripts to GitHub, so I definitely agree that we should try to analyze a wider corpus of scripts than those available on GitHub.

As for the addition of "extra complexity" to solve this problem: as far as wget and curl are concerned, this complexity will only affect those who have curl.exe installed on their machines, which I'd argue is a demographic that would understand we're aliasing to something that's definitely not "true curl" (though I'd gladly be proven wrong on this point if additional data shows otherwise; furthermore some more novice users of Git Bash aren't going to immediately understand what's going on).

More generally:

  1. Traditionally, much of the PowerShell Team has maintained a hardline approach to not making breaking changes (except on accident). As a few of you have brought up, now that we need to reconcile concepts across platforms, we probably need a mechanism for introducing smaller breaking changes over time. We should definitely write an RFC to address this (no ETA though). Specifically, I really like the idea of doing something like Python's future module, but we should definitely have a broader conversation on it.

My personal random thoughts and opinions:

P.S. If it's not super obvious, I'm the kind of person who likes to write longer-form, thought-out responses. That's why you'll sometimes see me coming in a bit later in the discussion, but I promise I'm doing my best to think critically about the problem space. :)

rkeithhill commented 7 years ago

@joeyaiello

should be able to copy/paste what they're doing interactively into a script, and it should work the same

Yes, on their machine but that doesn't mean that same script will run fine when deployed elsewhere. Another PowerShell user may have redefined an alias in "their" profile (or removed it) and perhaps in the case of a weak alias, have curl.exe installed (whereas I didn't). But this speaks to the different use cases of PowerShell. If the script is only for me and nobody else, I'll never have a problem. If I'm a module developer who expects thousands of different users to install and use my module - well, I have to be much more rigorous.

Regarding ScriptAnalyzer's AvoidAliases rule, I was one of those in favor of a whitelist for things like cd, foreach, where, select, sort and group. But given the new reality, I'm willing to scrap even those aliases. Although, man I have to say cd is a tough one to give up but .. what the heck. It's for the greater good. :-)

BTW the big issue with some of those rules in VSCode/PowerShell was that the rule's extent was way too large. Some of that has been fixed and as we get more of that fixed, it becomes less "visually painful" to enable more rules.

DerpMcDerp commented 7 years ago

I like the weak references proposal (and the "Foo -> Get-Foo" stuff) but I think PowerShell should have a simple/terse mechanism like:

enum CommandType {
    Alias
# Filter should be subsumed into Function
    Function
    Cmdlet
    Application
    ExternalScript
# I'm not sure what to do with Workflow and Configuration
    Script
}

&& <CommandType[]> <string> [optional-parameters]

to control the order of lookup or even exclude places from lookup. e.g.

<#PS#> && Application,Alias curl http://github.com

Searches for "curl" first in executable paths and if it isn't found searches for it in the alias list.

KevinMarquette commented 7 years ago

@joeyaiello I think we can agree that not all aliases are created equal. There is a class of alias that should be more acceptable and a class that should be strongly discouraged. It may be worth it to group the aliases and create stronger rules for the more conflicting ones like wget.

Your comment about copy/paste scripts working by default is an important one. I have seen many users where there workflow is to copy/paste a script into a PowerShell window on a different system. They are at that very early period of scripting where they know enough to be making a script but have not taken that step. They are also more likely not not have best practices or script analyzer guiding them yet. They are also more likely to be over using aliases.

We also can't overlook the weak alias scenario where someone can install wget and break scripts that have been rock solid and working. Or move them to another system where wget is already installed and have no idea what is wrong. Last thing I would want is to have code that looks like it is correct and not work. I cannot thing of anything more frustrating as a developer than fighting code that looks correct. When they go online for help, most people will find the script working for them. Also consider how the user with the broken script would figure it out.

I would much rather see them get the message The term 'wget' is not recognized as the name of a cmdlet, then search that message to find a healthy discussion of what we did. Then find that they need to replace the wget with invoke-webrequest, recreate the alias or start PowerShell with a flag. It is one thing to have a breaking change that people can figure out and another thing to have unusual logic that requires someone versed in PowerShell to point out.

Jaykul commented 7 years ago

I think weak aliases would further complicate the already complicated concept of command resolution in PowerShell, without solving the underlying problem: people feel like you deliberately stepped on native commands.

If there was a up or down vote on this RFC, I would vote no. A new type of "alias" is confusing, and doesn't resolve the underlying issue.

Note: there is a well established and well documented command precedence in PowerShell. The documentation includes explanations of how to see and invoke "hidden" or shadowed items, including how to use the call operator to run only an application or function and ignore an alias. I wouldn't necessarily be opposed to adding a helper to make that easier, but I want to be clear that it's not necessary.

Since it's come up, I would rather see all those aliases that were originally put there for learning purposes (i.e. to help linux or cmd users find the PowerShell version of their commands) be gradually deprecated and removed. I would ideally like to see them immediately moved into modules (which could be initially loaded like PSReadLine), so that we could "remove" them ourselves right away (and then, yes, use warnings, and event logs, and take a few releases to get it finished).

Once you figure out what you're going to do, I'd also like to see a simple "Oops, we're sorry, we didn't mean to make it harder to use curl" and "we're going to do our best not to obscure native tools in the future" blog post -- even though it's obvious some people will not believe you.

Additionally, perhaps Microsoft ought to ship a module or command to resolve, expand, and replace the aliases with (optionally fully qualified) command names, something like the Expand-Alias command in my ResolveAlias or CodeFlow "Expander" module.

Oh, and for what it's worth, rather than a faux "app" module or "app" provider, I want to point out that all that's needed is Start-Process ... maybe we should give people a function like:

function app {
param([string]$native)
    Start-Process $native $args -NoNewWindow -Wait
}
SteveL-MSFT commented 7 years ago

Looks like the PowerShell Team be meeting on Tues (8/30) instead of today. As long as this RFC is open, continue posting comments. Thanks.