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

Extend pluginOptions to also cover gulp-sourcemaps #65

Merged
merged 3 commits into from
Sep 16, 2015
Merged

Extend pluginOptions to also cover gulp-sourcemaps #65

merged 3 commits into from
Sep 16, 2015

Conversation

narthollis
Copy link
Contributor

As per issue #64

Currently this both the options for sourcemap.init and sourcemap.write compressed into the same option.

I can't see any option conflicts upstream, so I don't think this would be an issue.

@chmontgomery
Copy link
Contributor

Thanks for the PR! Definitely on the right track. A couple things:

  • There does seem to be one overlap in init/write options: debug. I'm not entirely sure if people use it for init and not write or visa versa, however to be more explicit about this I think we should support each individually, e.g.:
main: {
  scripts: [
    './*.js'
  ],
  styles: [
    './*.css'
  ],
  options: {
    pluginOptions: {
      'gulp-sourcemaps': {
        init: {loadMaps: false, debug: true},
        write: {addComment: false},
        destPath: '../maps' // this is the string "maps" by default
      }
    }
  }
}
  • Also, I could see people needing to supply different options to scripts vs styles. In order to support this we would structure it how we structure other options, but in this case use defaults in case you want to do the same for both streams, e.g.:
main: {
  scripts: [
    './*.js'
  ],
  styles: [
    './*.css'
  ],
  options: {
    pluginOptions: {
      'gulp-sourcemaps': {
        init: {debug: true},
        write: {addComment: false},
        destPath: '../maps',
        scripts: {
            init: {loadMaps: false},
            write: {addComment: true} // overrides {addComment: false}
        },
        styles: {
            init: {loadMaps: true},
            destPath: 'maps' // overrides '../maps'
        }
      }
    }
  }
}

Let me know if you'll be able to look into these items or have different ideas

@narthollis
Copy link
Contributor Author

Yeah, this seams all quite reasonable.

I will get this sorted tonight and update the PR.

@narthollis
Copy link
Contributor Author

I have updated this pull request with the suggested chnages.

var sourcemapBaseOpts = defaults({}, bundle[BundleKeys.OPTIONS].pluginOptions['gulp-sourcemaps']);
// Remove the style and script sub attributes
delete sourcemapDefaults[BundleKeys.SCRIPTS];
delete sourcemapDefaults[BundleKeys.STYLES];
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you mean sourcemapBaseOpts here instead of sourcemapDefaults?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, i thought i had fixed that typo sorry. I had fixed that one localy.

Copy link
Contributor

Choose a reason for hiding this comment

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

np, I just quick fixed it and merged with my changes

@chmontgomery
Copy link
Contributor

this change has landed in v2.23.0. Thanks for contributing!

@chmontgomery
Copy link
Contributor

please try it out and let me know if there are any issues

@narthollis narthollis deleted the sourcemap-pluginoptions branch September 16, 2015 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants