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

Apply style guide #2916

Merged
merged 52 commits into from
May 12, 2021
Merged

Conversation

henrikt-ma
Copy link
Collaborator

Fixes #2713.

This is the follow-up to #2884, where the style guide was created. This PR is about getting the document more in line with that style guide.

Currently in Draft state to reflect that there are still rules in the style guide that I plan to apply as part of this PR. Note that making the document comply with the style guide in every aspect is beyond the scope of this PR (and not expected to be in the scope of any specific PR in the future either). For example, I have no intention of finding and fixing all occurrences of non-American English.

@henrikt-ma henrikt-ma marked this pull request as draft April 11, 2021 21:05
@henrikt-ma
Copy link
Collaborator Author

How about extending the style guide with a rule for always having a comma after the abbreviations i.e. and e.g.?

@henrikt-ma
Copy link
Collaborator Author

Opinions on top level vs top-level?

chapters/classes.tex Outdated Show resolved Hide resolved
@@ -579,7 +580,7 @@ \section{Redeclaration}\label{redeclaration}
A \lstinline!redeclare! construct as an element replaces the declaration of a local class or component with another declaration.
Both \lstinline!redeclare! constructs work in the same way.
The \lstinline!redeclare! construct as an element requires that the element is inherited, and cannot be combined with a modifier of the same element in the extends-clause.
For modifiers the redeclare of classes uses a special short-class-definition construct; that is a subset of normal class definitions and semantically behave as the corresponding class-definition.
For modifiers the redeclare of classes uses a special short class definition construct; that is a subset of normal class definitions and semantically behave as the corresponding class-definition.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly. I believe we here should consistently view class-definition and short-class-definition as the grammar constructions; except I don't know what subset should be replaced with.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed apart from the subset question. Please check this one extra carefully, as I also made some small improvements to language.

What about calling it a special case rather than a subset?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Special case sounds good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. To avoid repeating the word special I removed it before short-class-definition. Please resolve if good.

henrikt-ma and others added 20 commits May 11, 2021 23:33
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
@henrikt-ma
Copy link
Collaborator Author

henrikt-ma commented May 11, 2021

Looking good except for the many minor comments, in particular I don't like sentences starting with a non-capital letter even if part of inline-listing and prefer working around that.

I applied your suggestions, even though I think the other variant reads better. It would be great if we could get at least a third opinion on this, but we can open a separate style guide issue for this to get more input from the group. Going back to the other variant should be very easy in case that is the outcome of group discussion.

Edit: It's only the ones starting with The that I don't like. The ones starting with A read better now than before.

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.

Looking good now.

@HansOlsson HansOlsson merged commit 03f0c28 into modelica:master May 12, 2021
@henrikt-ma henrikt-ma deleted the cleanup/apply-style-guide branch May 12, 2021 09:53
HansOlsson added a commit to HansOlsson/ModelicaSpecification that referenced this pull request Aug 9, 2021
HansOlsson added a commit that referenced this pull request Aug 10, 2021
Correct double for introduced in #2916
@HansOlsson HansOlsson added the M36 For pull requests merged into Modelica 3.6 label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M36 For pull requests merged into Modelica 3.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unusual way of marking language terms using hyphen instead of \lstinline
2 participants