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

Alias ps1 filetype to powershell #3010

Merged
merged 1 commit into from
Aug 13, 2020
Merged

Conversation

kevinoid
Copy link
Contributor

Thanks for writing a linter for PSScriptAnalyzer @zigford! I was surprised that it isn't used by default with vim-ps1 due to requiring manual configuration to alias the ps1 filetype to powershell. Is there a reason you chose not to include it in s:default_ale_linter_aliases?

Since vim-ps1 is a popular (the only?) PowerShell ftplugin and there do not appear to be any other uses of the ps1 filetype on vim.org, this seems like a safe and reasonable default. This PR would add it, assuming there wasn't a specific reason to avoid it.

Thanks again,
Kevin

Rather than requiring users to alias ps1 to powershell themselves,
include it in s:default_ale_linter_aliases.  Since [vim-ps1] is a
popular (the only?) PowerShell ftplugin and there do not appear to be
any other uses of ft=ps1 on vim.org, this seems like a safe and
reasonable default.

[vim-ps1]: http://www.vim.org/scripts/script.php?script_id=1327

Signed-off-by: Kevin Locke <[email protected]>
@zigford
Copy link
Contributor

zigford commented Feb 26, 2020

Hi @kevinoid I used powershell over ps1 as I didn't agree on vim-ps1's decision to use ps1 as the filetype name of powershell scripts. It doesn't fit with how every other language names their filetypes. Ie, python filetype is not named after it's extension, nor is ruby.

I've also made https://github.com/zigford/vim-powershell in the hopes that others will contribute and we can have sane powershell + vim functionality.

Anyway, I digress. The proper answer is that I did not know about s:default_ale_linter_aliases, so I think this will be a welcome change and make the current powershell linting easier to discover in ale.

@kevinoid
Copy link
Contributor Author

Thanks @zigford! Makes sense to me. I'm looking forward to checking out vim-powershell.

@w0rp: I think it's safe to merge. (If you approve, of course.)

@stale
Copy link

stale bot commented Aug 13, 2020

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

@stale stale bot added the stale PRs/Issues no longer valid label Aug 13, 2020
@kevinoid
Copy link
Contributor Author

@w0rp I think this PR is good to merge and not stale.

@stale stale bot removed the stale PRs/Issues no longer valid label Aug 13, 2020
Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is good. The bot is doing its job correctly.

@w0rp w0rp merged commit 0b77766 into dense-analysis:master Aug 13, 2020
@w0rp
Copy link
Member

w0rp commented Aug 13, 2020

Cheers! 🍻

@kevinoid
Copy link
Contributor Author

Yes indeed. Thanks for reviewing and merging it! 🥂

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.

3 participants