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] Quote environment variables in generated command #1541

Merged
merged 2 commits into from
Jul 7, 2022

Conversation

CommandMC
Copy link
Collaborator

Small thing, but we weren't quoting environment variables in the getCommand function, leading to commands like this:

WINEPREFIX=/home/commandmc/Games/Heroic/Prefixes/Akalabeth - World of Doom WINEFSYNC=1 ...

That's now fixed:

WINEPREFIX="/home/commandmc/Games/Heroic/Prefixes/Akalabeth - World of Doom" WINEFSYNC=1 ...

This is not a change relating to how commands are ran, this is just how they're represented


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@Nocccer
Copy link
Collaborator

Nocccer commented Jul 1, 2022

Please fix quoteIfNecessary with this

function quoteIfNecessary(stringToQuote: string) {
  if (
    (stringToQuote.charAt(0) !== '"' ||
      stringToQuote.charAt(stringToQuote.length - 1) !== '"') &&
    stringToQuote.includes(' ')
  ) {
    return `"${stringToQuote}"`
  }
  return stringToQuote
}

Else some values will be double quoted if there already quoted.

@CommandMC
Copy link
Collaborator Author

Done
I'm not quite sure how you'd get a double-quoted value here though, especially if we combine this with your env var & wrapper changes

@Nocccer
Copy link
Collaborator

Nocccer commented Jul 6, 2022

Done I'm not quite sure how you'd get a double-quoted value here though, especially if we combine this with your env var & wrapper changes

You get this easy if someone is "clever" enough (like me :D) to quote the envs already in the frontend. And that happend in the old solution aswell. You always put quotes around strings with spaces, without checking if there are already quoted.

string = "some string with sapces"
quoteIfNecessary(string) returns ""some string with sapces""

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

Looks good!

@CommandMC CommandMC merged commit 9f83b76 into main Jul 7, 2022
@CommandMC CommandMC deleted the fix/quote-env-vars branch July 7, 2022 13:17
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.

None yet

3 participants