Skip to content

add function-parameters contrib_syntax / install extensions if in after#266

Open
borisdaeppen wants to merge 2 commits into
vim-perl:devfrom
borisdaeppen:dev
Open

add function-parameters contrib_syntax / install extensions if in after#266
borisdaeppen wants to merge 2 commits into
vim-perl:devfrom
borisdaeppen:dev

Conversation

@borisdaeppen
Copy link
Copy Markdown

No description provided.

@borisdaeppen
Copy link
Copy Markdown
Author

Hello,

I hope it's okay that both commits are in the same pull request.

commit: add function-parameters contrib_syntax
Should be a nobrainer, this line is just missing

commit: also install extensions if in after
With this commit you can the do the following

make moose dancer
make install

and moose and dance will also get installed.
Before you had to copy them manually.

@borisdaeppen borisdaeppen changed the title add function-parameters contrib_syntax add function-parameters contrib_syntax / install extensions if in after Oct 6, 2018
@borisdaeppen
Copy link
Copy Markdown
Author

Please let me know if it is actually desirable to use install to also copy extensions (as my commit does).
Also, should I better use ifeq (Makefile) to make the test if folder exists instead of if [ -d (bash)?

Thx for any feedback to this PR

@hoelzro
Copy link
Copy Markdown
Member

hoelzro commented Oct 9, 2018

Hi @borisdaeppen! Thanks for the contribution, but I do have some reservations about the install rule - do you mind if I just merge 52b59f5?

@borisdaeppen
Copy link
Copy Markdown
Author

I don't mind. I already thought myself, the solution might not be optimal.

Might I ask if you see any validity in the basic idea? How about a separate rule e.g. installcontrib?

I would appreciate if you could formulate your reservations, if possible. So that I can improve the commit.

@hoelzro
Copy link
Copy Markdown
Member

hoelzro commented Oct 9, 2018

@borisdaeppen I think the base idea is ok - my main reservations come from one make rule setting up an after directory and a separate rule to install them into a user's ~/.vim directory with no notion of the dependencies between those two actions. It would be easy to run into a situation in which a user does make function-parameters followed by make install, updates the repository at a later time, and does another make install. I think many users would expect any changes to function-parameters.vim to be installed to, but they wouldn't. I'm not really sure why there are different make rules for the various contrib scripts; that feels more like something a configure script would be used for. But, IMO, a configure script for vim-perl would be overcomplicating things!

To be honest, now that Vim itself has a package manager, I kind of question the value of the makefile at all, except for maintainer-level tasks like building a distribution tarball or running the tests.

Comment thread README.md
$ ls after/syntax/perl
dancer.vim moose.vim

This local copy of after will be concidered by a later `make install`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo: concidered -> considered

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