-
Notifications
You must be signed in to change notification settings - Fork 25
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
Quick attempt at fixing multicharacter identifiers #2865
Conversation
@balacij, you are correct that the changes don't do much for the NFLGTFLSF problem, but I do think they are still an incremental improvement in the appearance of the formatting. If @JacquesCarette is okay with how you got it to work, I'm certainly okay with the what it looks like. 😄 How hard would it be to change the symbols with dashes in their names to use underscores instead? For instance, change is-safeLoad to is_safeLoad? The dash really does look like a minus sign when the LaTeX is compiled. This isn't something to spend much time on, but it feels like Drasil only has the name in one spot, so changing the name, should only involve changing it on one spot? (and changing stable, of course). For NFLGTFLSF I don't think it will look right unless we put a multiplication operator between the symbols (like |
Oops, saw the commit, but not the PR. So what's odd about this fix is that 1-letter identifiers will be treated differently than multi-letter. We should be consistent with out use of fonts. It's LaTeX's fault that it does something odd. So we should pick a font for our identifiers, and use it systematically (even though that might produce output that is not fully what would be written by hand). We can subsequently deal with things like multiplication, which is indeed a completely different issue. |
Thank you both for checking this PR 😄
Sounds good!
I doubt it would take much time at all 😄, I can include it in this PR (or in a new one if that's preferred).
I think the general rule is that multiplication of multicharacter variables looks ambiguous if at least 2 consecutive expressions are just plain identifiers, or if the multiplication of identifiers cause a name collision (multicharacter variables/identifiers potentially colliding with multiple shorter variables/identifiers. For example, if we have variables "ABC", "A", "B", and "C", and then we multiply "A", "B", and "C", we end up with The only ambiguous area I could think of with using either
I thought that all text written in a math environment (between Though, now that I think about single character identifiers placed in math environments, this would mean that we shouldn't need Aside: If we decide to always place identifiers in
Sounds good. |
The |
The results look good to me. @balacij, I think you hit on the reason why the underscore wasn't used previously. LaTeX doesn't like that symbol. I'm glad you were able to get it to work. This is certainly an improvement. I think we should probably postpone looking at how to make the multiplication look better. With respect to everything being in a math font inside an equation, not everything defaults to an italic font. Standard functions (like The use of |
Thank you, I agree.
I was not aware of these TeX rules. That's very interesting! Thank you 😄 |
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.
Not so much changes as explanation.
We should really ask ourselves whether we want to let people use |
The |
Personally, I have no problems with Regarding rules, what do we think about disallowing the characters used in the Expr/ModelExpr operators (e.g., \cdot, integrals, etc) to make expressions completely unambiguous? Though, I think I might have seen \sum/big sigma used as a variable name before. |
For GlassBR, I think going to camlCase will be simplest. Yes, certain operators should be disallowed. And yes, some Greek letters do get overloaded. But that should be done via using the shorthands for them, rather than directly hacking in the unicode. |
Updated now to use camel case. There is still the related task of creating rules for which characters are allowed in symbol names, but I think that can be done in a separate ticket. |
A follow-up from a previous discussion (#2856 (comment)). This PR is intended to showcase the simple attempt for a fix. It doesn't fix all the problems, but it makes the multi-character identifiers look a bit better in the variable description tables. However, the specific example you noted in #1853 (GlassBR's DD: calOfCapacity) is still problematic (almost no change in rendering). For that, we might need to add a small space (via
\
, or, preferably, something smaller) between identifiers whenever there is a group of identifiers directly next to each other (for example, terms with multiplication).