The identifiers in Sightly should always be lowercase only
Closed this issue · 4 comments
As the identifiers in Sightly (at least the one for the template identifier) are internally converted to lowercase, they should only be defined in lowercase in the first place. Otherwise one might run into collisions when defining a template named "myTemplate" and "mytemplate" in the same script, because to Sightly those identifiers are the same.
Reason is primarily that Sightly logs everything with the internal (lowercased) identifiers and finding the appropriate location in the Sightly script is much easier, if the identifier is also used in lower case in the script.
While it is correct that internally, the Sightly identifiers are lower case, this should stay an implementation detail. Despite myTemplate and mytemplate causing a naming collision, it would be bad practice in the first place to have variables names that only differ by character case.
I think that there really should be a way to distinguish words in identifiers, and keeping everything together in lowercase would make it much harder to read (just like "Donaudampfschifffahrtsgesellschaftskapitän"). As hyphens are not allowed characters for identifiers, there are only two possibilities left: camel case or underscores.
<sly data-sly-use.fooBar="com.package.FooBar">${fooBar.memberName}</sly>
<sly data-sly-use.foo_bar="com.package.FooBar">${foo_bar.memberName}</sly>In order to be consistent with the usual Java/JavaScript naming conventions, of member variables and getters, I'd recommend to use camel case consistently. Additionally, just like writing everything in lower case, this convention would also avoid naming collisions like myTemplate and mytemplate, since mytemplate wouldn't comply.
My preference still goes to camel-cased identifiers. If you stick to the style guide and use camel case consistently, you should not run into any naming collisions.
@kwin do you have more use cases where finding identifiers is difficult because they are converted to lowercase? Does this really happen that often that it's worth to sacrifice the readability of all Sightly scripts?
Ok, I agree. I only stumbled around the warning of duplicate global variables (which uses the internal identifiers) and couldn't find the according place in the code. But I agree that readibility is worth more than that and if you are aware of that limitation, everything should be fine. I will revert that commit from my PR.
Ok, thank you @kwin . I'll close this issue.