Adding support for --force-ext option. #140

Merged
merged 1 commit into from Feb 23, 2013

Conversation

Projects
None yet
4 participants
@MaxPower15

Allows the user to specify an extension on the commandline. When present, all input files will be processed as if they have this extension.

This is useful if you have filenames with multiple sequential extensions, or just a non-standard extension that cannot be changed for one reason or another.

For example, I have quite a few files which have .coffee.erb extensions. These files are 99.9% Coffeescript and can be parsed as such, but having the .erb suffix is required for our rendering engine. With this change, I can process them like so:

docco --force-ext .coffee path/*.coffee.erb
Adding support for --force-ext option.
Allows the user to specify an extension on the commandline. When
present, all input files will be processed as if they have this
extension.
@jashkenas

This comment has been minimized.

Show comment Hide comment
@jashkenas

jashkenas Jan 18, 2013

Owner

Doesn't it already work if you pass the explicit path for each file to the compiler?

Owner

jashkenas commented Jan 18, 2013

Doesn't it already work if you pass the explicit path for each file to the compiler?

@MaxPower15

This comment has been minimized.

Show comment Hide comment
@MaxPower15

MaxPower15 Jan 18, 2013

Here's my output if I target a specific file:

➜  v1 git:(master) ✗ docco video.js.coffee.erb
docco: skipped unknown type (video.js.coffee.erb)

As far as I know, the only way to accomplish this without --force-ext is to copy the target file to a tmp file with an extension it recognizes. That method works, but not needing tmp files or directories would be nice.

Here's my output if I target a specific file:

➜  v1 git:(master) ✗ docco video.js.coffee.erb
docco: skipped unknown type (video.js.coffee.erb)

As far as I know, the only way to accomplish this without --force-ext is to copy the target file to a tmp file with an extension it recognizes. That method works, but not needing tmp files or directories would be nice.

@jashkenas

This comment has been minimized.

Show comment Hide comment
@jashkenas

jashkenas Jan 18, 2013

Owner

Oh, I'm sorry -- this is a Docco ticket. I thought it was a CoffeeScript ticket ... got it.

Owner

jashkenas commented Jan 18, 2013

Oh, I'm sorry -- this is a Docco ticket. I thought it was a CoffeeScript ticket ... got it.

@mlangenberg

This comment has been minimized.

Show comment Hide comment
@mlangenberg

mlangenberg Feb 21, 2013

+1

Just ran into this. Rails' asset pipeline makes it easy to have chain of app.js.coffee.erb, I would like to use Docco on this.

Any quick workaround until this is getting merged?

+1

Just ran into this. Rails' asset pipeline makes it easy to have chain of app.js.coffee.erb, I would like to use Docco on this.

Any quick workaround until this is getting merged?

@@ -216,7 +216,11 @@ for ext, l of languages
l.docsSplitHtml = ///<h1>#{l.name}DOCDIVIDER</h1>///
# Get the current language we're documenting, based on the extension.
-getLanguage = (source) -> languages[path.extname(source)]

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 21, 2013

Collaborator

I might keep this inline and do something like:

 getLanguage = (source, config) -> languages[config.forceExt or path.extname(source)]

config is always set when running through code, so there should be no need to check for its existence.

@justindujardin

justindujardin Feb 21, 2013

Collaborator

I might keep this inline and do something like:

 getLanguage = (source, config) -> languages[config.forceExt or path.extname(source)]

config is always set when running through code, so there should be no need to check for its existence.

@@ -271,6 +276,7 @@ run = (args=process.argv) ->
.option("-c, --css [file]","use a custom css file",defaults.css)
.option("-o, --output [path]","use a custom output path",defaults.output)
.option("-t, --template [file]","use a custom .jst template",defaults.template)
+ .option("--force-ext <ext>","treat the target files as if they have a specific extension",defaults.forceExt)

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 21, 2013

Collaborator

For the sake of consistency, I would add a short flag for this. Perhaps -f ?

@justindujardin

justindujardin Feb 21, 2013

Collaborator

For the sake of consistency, I would add a short flag for this. Perhaps -f ?

@@ -36,6 +36,12 @@ test "custom CSS file", ->
["#{testPath}/*.coffee"],
css: "#{resourcesPath}/pagelet.css"
+# **Specifying a filetype independent of extension should be supported**

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 21, 2013

Collaborator

+1 for having a test.

@justindujardin

justindujardin Feb 21, 2013

Collaborator

+1 for having a test.

@justindujardin

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 21, 2013

Collaborator

I added some cleanup comments to this pull. The functionality looks good to me, seems valuable, and has a test to make sure it works. 👍

Collaborator

justindujardin commented Feb 21, 2013

I added some cleanup comments to this pull. The functionality looks good to me, seems valuable, and has a test to make sure it works. 👍

@jashkenas

This comment has been minimized.

Show comment Hide comment
@jashkenas

jashkenas Feb 22, 2013

Owner

The cleanups look like they should be used ... I'm also iffy on the naming. force-extension is a bit strong language, when there's nothing really being forced.

Owner

jashkenas commented Feb 22, 2013

The cleanups look like they should be used ... I'm also iffy on the naming. force-extension is a bit strong language, when there's nothing really being forced.

@justindujardin

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 22, 2013

Collaborator

Perhaps something like:

-e, --extension 'assume a specified extension for files, rather than auto detection'

Collaborator

justindujardin commented Feb 22, 2013

Perhaps something like:

-e, --extension 'assume a specified extension for files, rather than auto detection'

@MaxPower15

This comment has been minimized.

Show comment Hide comment
@MaxPower15

MaxPower15 Feb 22, 2013

Cool guys, I'm glad to see this get some traction. The suggested changes all look good to me. Would you like me to make the changes directly to this branch, or will you clean it up on your end?

Cool guys, I'm glad to see this get some traction. The suggested changes all look good to me. Would you like me to make the changes directly to this branch, or will you clean it up on your end?

@justindujardin

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 22, 2013

Collaborator

We're all pretty busy over here--it'd be best if you made the changes yourself. Thanks for your contribution.

Collaborator

justindujardin commented Feb 22, 2013

We're all pretty busy over here--it'd be best if you made the changes yourself. Thanks for your contribution.

@justindujardin justindujardin merged commit ed28d2f into jashkenas:master Feb 23, 2013

@justindujardin

This comment has been minimized.

Show comment Hide comment
@justindujardin

justindujardin Feb 23, 2013

Collaborator

@MaxPower15 I had a few minutes, so I implemented your changes with the cleanups we've talked about.

I went with the following command line specification:

-e, --extension <ext>  use the given file extension for all inputs

Now you can do things like:

docco -e .coffee Cakefile src/docco.coffee

Thanks for your contribution, this also invalidates #121

Collaborator

justindujardin commented Feb 23, 2013

@MaxPower15 I had a few minutes, so I implemented your changes with the cleanups we've talked about.

I went with the following command line specification:

-e, --extension <ext>  use the given file extension for all inputs

Now you can do things like:

docco -e .coffee Cakefile src/docco.coffee

Thanks for your contribution, this also invalidates #121

@MaxPower15

This comment has been minimized.

Show comment Hide comment
@MaxPower15

MaxPower15 Feb 23, 2013

Awesome, thanks Justin. :)

Awesome, thanks Justin. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment