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

Variables related fixes #693

Merged
merged 10 commits into from Jun 6, 2016
Merged

Conversation

thuck
Copy link
Contributor

@thuck thuck commented Jun 2, 2016

This includes the check of the $ for the variable assigned.
So set something /home generates an error, since the correct syntax must be set $something /home.
This also should fix partially the issue #681 since now it handles the undefined behavior of p.we_wordv.

@@ -2152,6 +2152,10 @@ static struct cmd_results *cmd_set(int argc, char **argv) {
return error;
}

if (argv[0][0] != '$') {
return cmd_results_new(CMD_FAILURE, "set", "Malformed variable assignment, name has to start with $");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation issues here.

@ddevault
Copy link
Contributor

ddevault commented Jun 2, 2016

I would rather add the $ for them than raise an error.

@thuck
Copy link
Contributor Author

thuck commented Jun 2, 2016

From what I understood from the i3 code this is the default behavior, it only raises an error.
But I can change this if you think it would be more aligned with sway.

@ddevault
Copy link
Contributor

ddevault commented Jun 2, 2016

I think it would be better, yes. Log a warning though.

@thuck
Copy link
Contributor Author

thuck commented Jun 6, 2016

@SirCmpwn is it ok now? Or do you need any modification to merge this?

@ddevault ddevault merged commit 2e10f0a into swaywm:master Jun 6, 2016
@ddevault
Copy link
Contributor

ddevault commented Jun 6, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants