noseglid / atom-build

:hammer: Build your project directly from the Atom editor
https://atom.io/packages/build
MIT License
248 stars 97 forks source link

Pass resolved replacements to preBuild #510

Closed jordiorlando closed 7 years ago

jordiorlando commented 7 years ago

I want to selectively apply some {SELECTION} replacements in my preBuild function but I can't find a way to access those resolved strings. Would it be possible to pass those as arguments into preBuild? Perhaps the function signature could be changed to preBuild(replacements) where replacements is an object with properties file_active, etc that just contain the strings that these replacements would resolve to.

noseglid commented 7 years ago

I think it would make a whole lot sense to just evaluate them yourself. For e.g. {SELECTION}, it would be as simple as atom.workspace.getActiveTextEditor().getSelectedText(). The full resolved build configuration will be available in this.

Would this solve your issue?

jordiorlando commented 7 years ago

While that would work fine, I think it would be better to include this functionality in the core package. For one, the method you describe would require the user to monitor the atom-build package for any changes to match the code. I think there's something to be said for providing the exact replacements that will be used rather than requiring the user to guess. I implemented this in #511.

noseglid commented 7 years ago

You wouldn't need to do any further monitoring if you fetch the current selection every time preBuild is called.

I'd rather not commit to any more APIs than I have to. Every API is increased maintenance and an opportunity to disappoint users by changing it. Either intentional or unintentional.

I appreciate the effort you put into this and I thank you, but I will not be accepting.

jordiorlando commented 7 years ago

Okay, I completely understand! Just to clarify, by "monitor" I meant just keep track of how atom-build calculates the replacements. Because if the user's preBuild relies on what it thinks the replacements will be but then atom-build is changed such that e.g. FILE_ACTIVE has a slightly different format, their preBuild will break.

jordiorlando commented 7 years ago

@noseglid Sorry to bring this up again, but do you have a recommended way of calculating e.g. FILE_ACTIVE_PATH? I tried simply copy-pasting the atom-build code into my preBuild function but am getting an error "path.dirname is not a function". It seems that the path library isn't being imported in a manner that makes it accessible to preBuild. Any ideas or workarounds besides forking atom-build?

noseglid commented 7 years ago

You would have to import path or any other component you want. preBuild is simply called as any other function.

const path = require('path');
// or
import path from 'path';

should do it.

jordiorlando commented 7 years ago

Thank you, the import method did not work but require('path') did.