-
-
Notifications
You must be signed in to change notification settings - Fork 245
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
Sitemap editor: fix negative number parameters #3055
Conversation
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
#2784 Bundle Size — 10.98MiB (+0.01%).49f3338(current) vs 95844d2 main#2743(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
Current #2784 |
Baseline #2743 |
|
---|---|---|
1.9MiB |
1.9MiB |
|
577.21KiB |
577.21KiB |
|
17.51% |
17.51% |
|
227 |
227 |
|
250 |
250 |
|
2952 |
2952 |
|
154 |
154 |
|
1.8% |
1.8% |
|
98 |
98 |
|
2 |
2 |
Bundle size by type
7 changes
7 regressions
Current #2784 |
Baseline #2743 |
|
---|---|---|
9.19MiB (+100% ) |
undefined |
|
867.33KiB (+100% ) |
undefined |
|
526.1KiB (+100% ) |
undefined |
|
295.6KiB (+100% ) |
undefined |
|
140.74KiB (+100% ) |
undefined |
|
1.38KiB (+100% ) |
undefined |
|
871B (+100% ) |
undefined |
Bundle analysis report Branch mherwege:sitemap_number_param Project dashboard
Generated by RelativeCI Documentation Report issue
Would this work on browsers with a locale that uses a comma as the decimal point? |
This is about the sitemap editor and how it parses sitemap xtext syntax, not about displayed values. So, this is not impacted by locale. I assume you had our other discussion about input fields and uom in mind. |
Oh good point, it just needs to be the same number syntax that core will use to parse the sitemap. I assume core will require a dot, regardless of its locale? |
Yes, it does. the sitemap syntax does not depend on locale. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
See: https://community.openhab.org/t/basic-ui-setpoint-negative-values/161011
Parameters with negative number where not parsed correctly. This is because hyphen is defined as a token and therefore already recognized before recognizing the number. Changing the order does not help as it breaks chart period parsing.
This fix avoids recognizing numbers as tokens and handles numbers in the lexer.
Tests where added and all existing tests still pass.