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

Add support for subarg to allow passing file extension #65

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

Add support for subarg to allow passing file extension #65

wants to merge 1 commit into from

Conversation

cymen
Copy link

@cymen cymen commented Nov 20, 2014

Browserify supports passing arguments to transforms via subarg. This
commit adds support for that so we can use es6ify on the command line
like so:

browserify -t [es6ify --extension=.js.es6] input.js

@cymen
Copy link
Author

cymen commented Nov 20, 2014

I want to use es6ify as a transform with browserify-rails. So do some other people:

browserify-rails/browserify-rails#19

I asked about subargs here and there was some confusion:

#60

This PR adds support for subargs or passing the file extension via the command line. Another transform that has this pattern is hbsfy and browserify expects transforms to take arguments in this way:

Passing arguments to transforms and plugins:

  For -t, -g, and -p, you may use subarg syntax to pass options to the
  transforms or plugin function as the second parameter. For example:

    -t [ foo -x 3 --beep ]

@cymen cymen changed the title Add support for subargs to allow passing file extension Add support for subarg to allow passing file extension Nov 21, 2014
@cymen
Copy link
Author

cymen commented Nov 24, 2014

@thlorenz @domenic Anything I can do to make this good to merge? I'd love to be able to use this and am more than happy to change it if it's not quite right.

@cymen
Copy link
Author

cymen commented Dec 10, 2014

I fixed the test so it passes on CircleCI. I also forked to:

https://www.npmjs.com/package/es6ify-with-subarg

Seems silly to fork over such a small thing but this is blocking any chance of using it on my project and well, I want to use it. Looking forward to hopefully deleting the fork!

@thlorenz
Copy link
Owner

I'm sorry you had to create a fork cause we are too slow to get this in.
We were planning on adding transform args @guzart actually made some changes to that end.

We just need to consolidate this and come up with a good API to finally get this feature added and published.

@guzart @domenic what do you think of the implementation suggested in this PR? Is this close to what we intended to do as well?

@cymen
Copy link
Author

cymen commented Dec 10, 2014

@thlorenz Not a problem -- I only mentioned I forked it to take the pressure off (I noticed another PR seems to be in a similar area). I'd like to kill the fork ASAP but in the meantime it lets the team I'm on evaluate using it.

@thlorenz
Copy link
Owner

@cymen makes perfect sense I'm gonna be out of action also called vacation without a computer pretty soon, but hopefully either of the other two contributors can help get this feature in soon.

@domenic
Copy link
Collaborator

domenic commented Dec 12, 2014

Adding opts seems like a good idea but I don't like the extension option very much. Haven't thought too hard about it though.

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.

None yet

3 participants