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

Added psake (default) and pester 'compiler' options #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dragon788
Copy link

@dragon788 dragon788 commented Jun 29, 2016

Opening this mostly to get eyes on it. I'd like to put in some logic to detect if a user has Psake or Pester installed (ideally need both since Psake is just "make" and Pester is the test framework).

I'm not sure if this is best done with an install.ps1 in the root of the repo that does this check or if there is a way to simply bomb out if powershell -Command Get-Command Invoke-Pester returns an error.

Currently I'm using this quite happily with https://github.com/tpope/vim-dispatch and it's pretty awesome.

@@ -11,6 +11,8 @@ if exists("b:did_ftplugin") | finish | endif
" Don't load another plug-in for this buffer
let b:did_ftplugin = 1

compiler psake
Copy link
Collaborator

Choose a reason for hiding this comment

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

Quite a few assumptions are made here about the script, so I don't think we should default the compiler. By far, I would expect the majority of scripts to be just normal scripts psake would fail to execute. Besides, @PProvost has generally pushed back on putting any general user preferences in these files (and I generally agree) and let them define how they want them set. We could maybe add tips to the README.md, but even that may be outside the purview of this project.

Copy link
Author

Choose a reason for hiding this comment

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

I'm inclined to agree. Technically the compiler doesn't do anything unless
invoked by the user, but I can see how having it there could be potentially
confusing if the user is using a regular Makefile to drive their process
and suddenly it tries to invoke Psake/Pester. So you think just having the
compilers available and letting the user choose with :compiler psake would
be more friendly?

On Wed, Jun 29, 2016 at 6:00 PM, Heath Stewart [email protected]
wrote:

In ftplugin/ps1.vim
#17 (comment):

@@ -11,6 +11,8 @@ if exists("b:did_ftplugin") | finish | endif
" Don't load another plug-in for this buffer
let b:did_ftplugin = 1

+compiler psake

Quite a few assumptions are made here about the script, so I don't think
we should default the compiler. By far, I would expect the majority of
scripts to be just normal scripts psake would fail to execute. Besides,
@PProvost https://github.com/PProvost has generally pushed back on
putting any general user preferences in these files (and I generally agree)
and let them define how they want them set. We could maybe add tips to the
README.md, but even that may be outside the purview of this project.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/PProvost/vim-ps1/pull/17/files/7d520788b1337da4010839a87c94e73a3fed1784#r69043818,
or mute the thread
https://github.com/notifications/unsubscribe/AAdxXk0NbPNUVwc92yKLubg2lMjFlfClks5qQvkfgaJpZM4JBZbW
.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think adding the compiler support should be fine, though I'd like @PProvost to weigh in. Part of me feels like it's extraneous to the intent of this bundle.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think remove the compiler line and I'm inclined to take this.

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