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

Fix illegal replacement of double quotes by single quotes #179

Merged
merged 1 commit into from
Sep 11, 2013

Conversation

Seldaek
Copy link
Contributor

@Seldaek Seldaek commented Jan 29, 2013

No description provided.

@rodjek
Copy link
Owner

rodjek commented Jan 29, 2013

I would say that given the style guide's opinion on quotes, the correct thing to do in this circumstance would be to reverse the quotes

exec { 'sh -c "foo"': }

@Seldaek
Copy link
Contributor Author

Seldaek commented Jan 30, 2013

@rodjek yes and no. I mean the current example is simple of course, but in more complex cases using double quotes vs single quotes can be quite a pain due to escaping of vars and all. It can make stuff quite harder to read.

The real issue right now is that it switches to single quotes but leaves the single quotes in the string intact, resulting in an invalid string like: 'sh -c 'foo''.

So I would suggest merging this for now, then either:

  • patch it so it prints a warning again for those, but does not apply the broken fix (I can try and do that if my ruby allows).
  • patch it so it does the fix correctly, but that might prove hard to do correctly because it relies on shell escaping rules.

@hcgrove
Copy link

hcgrove commented Feb 7, 2013

As the shell command might contain code in pretty much any language, doing the fix correctly would pretty much require parsing any language (just imagine what it would take not to trash code building some code passed to eval).

I would say the only sane to do when nested quoting is found is nothing, perhaps print a warning, but please add an option to skip those.

@dalen
Copy link
Contributor

dalen commented Aug 5, 2013

I think geppetto fixes it into something like this:

exec { 'sh -c \'foo\'': }

Note that there is a issue to relax the style guideline in cases like this: http://projects.puppetlabs.com/issues/19294

rodjek pushed a commit that referenced this pull request Sep 11, 2013
Fix illegal replacement of double quotes by single quotes
@rodjek rodjek merged commit 41405bc into rodjek:fix_stuff Sep 11, 2013
bastelfreak pushed a commit to bastelfreak/puppet-lint that referenced this pull request Mar 22, 2024
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.

4 participants