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

GRADLE-3084 escaping of $ in unix scripts #422

Closed
wants to merge 1 commit into from
Closed

GRADLE-3084 escaping of $ in unix scripts #422

wants to merge 1 commit into from

Conversation

jscancella
Copy link

Added variable escapeMetaCharactersInDefaultJvmOpts to application plugin so that you are able to programmatically turn off the escape of the $ when creating unix run scripts. This is enabled by default, to disable and revert to old behavior set escapeMetaCharactersInDefaultJvmOpts = true in application closure

links to https://issues.gradle.org/browse/GRADLE-3084

…plication plugin so that you are able to programmatically turn off the escape of the $ when creating run scripts. This is enabled by default, to disable and revert to old behavior set escapeMetaCharactersInDefaultJvmOpts = true in closure
@bmuschko
Copy link
Contributor

Hi John,

Thanks for providing the pull request. You might have noticed that we are currently working on allowing to provide your own implementation of a script generator for the application plugin. That'll give users of the plugin far more flexibility with respect to

  • Providing your own custom implementation of a script generator.
  • Changing the default template file.

We certainly also want to allow for customizing the default script generator implementations at some point of time as provided by your pull request. Instead of exposing a way to customize the escape logic for a single meta character, we'd probably rather want to go for a more generic approach. A solution I can think of is a handler implementation that can be set for a script generator from the build script. It's implementation could contain any kind of escape logic you personally need.

I'll think of a good approach here and let you know. At the moment the code is still little bit in flux. Once it's more stable, I'll let you know.

@jscancella
Copy link
Author

Sounds good to me! Glad to know that something even better is coming down
the line. I personally ran into these issues when converting from ant/maven
so I thought it a good place to start. Let me know if/when I can help.

On Thu, Mar 19, 2015 at 1:17 PM, Benjamin Muschko notifications@github.com
wrote:

Hi John,

Thanks for providing the pull request. You might have noticed that we are
currently working on allowing to provide your own implementation of a
script generator for the application plugin. That'll give users of the
plugin far more flexibility with respect to

  • Providing your own custom implementation of a script generator.
  • Changing the default template file.

We certainly also want to allow for customizing the default script
generator implementations at some point of time as provided by your pull
request. Instead of exposing a way to customize the escape logic for a
single meta character, we'd probably rather want to go for a more generic
approach. A solution I can think of is a handler implementation that can be
set for a script generator from the build script. It's implementation could
contain any kind of escape logic you personally need.

I'll think of a good approach here and let you know. At the moment the
code is still little bit in flux. Once it's more stable, I'll let you know.


Reply to this email directly or view it on GitHub
#422 (comment).

~John Scancella

@pioterj pioterj added this to the waiting-for-commiter milestone Nov 12, 2015
@bmuschko
Copy link
Contributor

Thanks for providing the pull request, @jscancella. In March I mentioned that we wanted to provide support for changing the default template file. The code has been released and the API has stabilized. That'll likely fix your issue though it's less straight forward than what you propose.

Unfortunately, we still haven't had the chance to find a more generalized approach for escaping metadata characters. I wrote up a spec a while ago but it's still pending a review on our end. I'll try to move this forward and let you know when we think this is right way forward. I am going to close the PR for now. Thanks again for contributing!

@bmuschko bmuschko closed this Nov 16, 2015
@johnscancella
Copy link

@bmuschko I did see the work that you all put into the new script generation and have been using it (though for me it kinda defeats the purpose of having gradle generate the script if I supply a customized one to it. But maybe that is just me).

I just looked at your spec, and it is unclear to me if the default is still the logic of escaping meta-characters? If you are still defaulting to escaping the meta-characters I would just like to add my $0.02 that I believe that to go against the spirit of gradle (sensible defaults). That is not something I would have suspected, hence why this issue was raised.

thanks for looking into this issue, and I hope that I can contribute to an accepted solution in the future.

@bmuschko
Copy link
Contributor

@johnscancella

I did see the work that you all put into the new script generation and have been using it (though for me it kinda defeats the purpose of having gradle generate the script if I supply a customized one to it. But maybe that is just me).

What probably would make sense for you is to write a plugin that applies the application plugin plus any additional configuration you need e.g. custom templates so you can reuse it in multiple project.

I just looked at your spec, and it is unclear to me if the default is still the logic of escaping meta-characters?

We'd apply the same defaults we have in place at the moment. Usually we try to avoid breaking changes but give people the option of changing the defaults.

@ov7a ov7a removed this from the waiting-for-gradle-team milestone Mar 28, 2024
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.

None yet

6 participants