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

Don't use inline code for the Boolean values #3565

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

henrikt-ma
Copy link
Collaborator

To is how we do it in the vast majority of cases, and is consistent with not using code style for numeric values.

Inspired by what appears to be a styling mistake made in #3561. I checked for occurrences of the following to conclude that the style we use is plain English:

  • is true
  • is \lstinline!true!
  • is false
  • is \lstinline!false!

It should be noted that there is still a number of cases of just \lstinline!false! and \lstinline!true! that would be good candidates to also turn into plain English. However, there are also cases where I think it is clear that code style should be used, so it isn't as simple as turning them all into plain English. Here's an example of the latter:

The value of \lstinline!connectorSizing! must be a literal \lstinline!false! or \lstinline!true!.

Perhaps we should try to write down a some guideline in the style guide?

To is how we do it in the vast majority of cases, and is consistent with not using code style for numeric values.

Inspired by a styling mistake made in modelica#3561.
@HansOlsson
Copy link
Collaborator

To me it would intuitively make sense to also use inline code for Boolean values, if we actually write them.

I could understand if we made an exception so that "is false" would be ok as non-code -- with the interpretation that it is a general English "false", not "false" in Modelica. The downside of that is that it is too subtle.
However, as the 2nd change in overloaded.tex a better option for that is to skip it altogether. The problem is that it may require a larger rewrite.

But I don't see a similar case if used more directly (like the 1st change in that file).

Note that the C++-standard also has false and true as inline code.

@HansOlsson HansOlsson added this to the 2024-September milestone Aug 30, 2024
@eshmoylova
Copy link
Member

To me, if you talk about default values or values, it would make sense to use the inline false or true. So, I would prefer the style that we had before this change.

Copy link
Member

@eshmoylova eshmoylova left a comment

Choose a reason for hiding this comment

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

Except for one case, I would prefer the old style.

chapters/classes.tex Show resolved Hide resolved
chapters/overloaded.tex Show resolved Hide resolved
@henrikt-ma
Copy link
Collaborator Author

But I don't see a similar case if used more directly (like the 1st change in that file).

To me, this piece of text is general mathematics/logics, and should hence not be formatted as if it were Modelica.

Note that the C++-standard also has false and true as inline code.

Yes, I would expect any standard documents for a programming language with built-in literals for the Boolean values to at least sometimes make source code. However, I doubt that the C++ standards format every occurrence of true and false as source code.

@HansOlsson
Copy link
Collaborator

But I don't see a similar case if used more directly (like the 1st change in that file).

To me, this piece of text is general mathematics/logics, and should hence not be formatted as if it were Modelica.

I disagree. If we introduce an annotation shouldPass with values true and false, and discuss their meaning we are not discussing some abstract true/false concept but the Modelica-specific values true/false.

Note that the C++-standard also has false and true as inline code.

Yes, I would expect any standard documents for a programming language with built-in literals for the Boolean values to at least sometimes make source code. However, I doubt that the C++ standards format every occurrence of true and false as source code.

For the C++23 draft the first 26 occurrences of false are formatted as source code, but the 27th is non-source code.

The difference is that the 27th one states that when a relationship is false (in the English sense), the corresponding value is false, i.e., it is bridging the gap between the abstract world and the programming language specific one.

@henrikt-ma
Copy link
Collaborator Author

But I don't see a similar case if used more directly (like the 1st change in that file).

To me, this piece of text is general mathematics/logics, and should hence not be formatted as if it were Modelica.

I disagree. If we introduce an annotation shouldPass with values true and false, and discuss their meaning we are not discussing some abstract true/false concept but the Modelica-specific values true/false.

What does this have to do with shouldPass?

The vector $P$ indicates whether argument $m$ of \lstinline!f! has a default value (true for default value, false otherwise).

@henrikt-ma
Copy link
Collaborator Author

The difference is that the 27th one states that when a relationship is false (in the English sense), the corresponding value is false, i.e., it is bridging the gap between the abstract world and the programming language specific one.

So they got it right!

@HansOlsson
Copy link
Collaborator

HansOlsson commented Sep 4, 2024

I disagree. If we introduce an annotation shouldPass with values true and false, and discuss their meaning we are not discussing some abstract true/false concept but the Modelica-specific values true/false.

What does this have to do with shouldPass?

I mixed up which commit in the PR was discussed here, and as that was the proposed change in annotations.tex I assume you agree that it shouldn't be made, right?

The vector $P$ indicates whether argument $m$ of \lstinline!f! has a default value (true for default value, false otherwise).

This depends on whether we discuss P as having values according to Modelica or in some other sense. The current specification clearly has P with values according to Modelica.

According to today's phone meeting decision.
@henrikt-ma
Copy link
Collaborator Author

Updated according to today's phone meeting. Re-requesting reviews.

Copy link
Collaborator

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Ok

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

Successfully merging this pull request may close these issues.

3 participants