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

theme variable values with numbers and operands can't be interpreted as strings #429

Closed
asilvestre opened this issue Apr 28, 2016 · 4 comments
Assignees
Labels

Comments

@asilvestre
Copy link

With a theme like this:

vars:
    date: "01-04-2016"
footer:
    right: $vars_date

I get -2019 instead of 01-04-2016

@mojavelinux mojavelinux added the bug label May 1, 2016
@mojavelinux mojavelinux added this to the v1.5.0.alpha.12 milestone May 1, 2016
@mojavelinux mojavelinux self-assigned this May 1, 2016
@mojavelinux
Copy link
Member

mojavelinux commented May 1, 2016

This is where our custom expression language is bleeding through, which itself is an attempt to cover the limitations of YAML. The date is being interpreted as a math expression (1 - 4 - 2016).

The quickest way to prevent dates from being confused with a math expression is to require a space on either side of the operator. The theming guide already states this as a requirement, so we'd simply being enforcing it. I'll send a PR with that change.

However, there's still a chance that other patterns inside content get mixed up. Therefore, another change we could make is to not run any key with a content segment through the evaluate_math function. That seems to make sense. I can't see why you'd want content to be a math expression.

The problem that remains are variable values. The value of an arbitrary variable is still going to get run through the evaluate_math function (since we don't know what it's used for). We'd need to come up with a way to tell the theme loader to treat a value as a string literal (no math). We could use the convention of spreadsheets and prefix the value with a single quote.

(expr.start_with? '\'') ? (expand_vars expr[1..-1], vars) : evaluate_math(expand_vars expr, vars)

@mojavelinux
Copy link
Member

I implemented both the enforcement of whitespace on either side of a math operator and skipped evaluating math in content-related keys. I have not yet implemented a way to indicate that a value should be treated as a string literal (variable expansion only). Let me know if you think we should proceed with that idea.

@asilvestre
Copy link
Author

Thanks! For me with these changes it's good enough.

@mojavelinux
Copy link
Member

Super! I'll file a separate issue to discuss the idea of having a string literal (variable expansion only) for the value.

Thanks for testing!

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

No branches or pull requests

2 participants