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

Draft: Improveme Documentation #1023

Closed
wants to merge 1 commit into from

Conversation

incaseoftrouble
Copy link
Contributor

fixes #999

Changes:

  • Relative links in the Markdown
  • Semantic one-line=one-sentence formatting
  • Prominent distinction between BenchExec, benchexec, and runexec
  • Brief usage instructions for both tools
  • Separate python API doc file
  • Restructure the benchexec description

Probably not finished yet, but maybe give it a read.

@PhilippWendler
Copy link
Member

Thanks!

The mixture of technical changes, new text, improvements, and move of text all in one commit makes it somewhat hard to review, though. For example, it is not visible whether the moved text is changed at the same time or not. Would it be possible to split the commit from this PR into independent commits, one for each part of the PR? (as identified in the PR description)

Semantic one-line=one-sentence formatting

This is something that we prefer to keep as it was: lines should not be too long, and thus line breaks should be added at semantically fitting places (like clauses of a sentence). Short lines make it easier to work with the text, for example when looking at diffs. So I would like to ask you to revert these changes and write new text in the same style as the existing text.

@incaseoftrouble
Copy link
Contributor Author

This is something that we prefer to keep as it was: lines should not be too long, and thus line breaks should be added at semantically fitting places (like clauses of a sentence). Short lines make it easier to work with the text, for example when looking at diffs. So I would like to ask you to revert these changes and write new text in the same style as the existing text.

To be honest, I would need a sensible rule to do that. I tried to infer something from the existing text but I couldn't find something that generally worked. I can try in a best-effort way.

Would it be possible to split the commit from this PR into independent commits, one for each part of the PR? (as identified in the PR description)

Most of these mutually depend on each other. I can give it a try but I don't see too many possible splits.

@PhilippWendler
Copy link
Member

This is something that we prefer to keep as it was: lines should not be too long, and thus line breaks should be added at semantically fitting places (like clauses of a sentence). Short lines make it easier to work with the text, for example when looking at diffs. So I would like to ask you to revert these changes and write new text in the same style as the existing text.

To be honest, I would need a sensible rule to do that. I tried to infer something from the existing text but I couldn't find something that generally worked. I can try in a best-effort way.

Simply keep the lines not too long (~80 characters is good) and break the lines at those places where it matches the sentence structure, i.e., prefer line breaks between two clauses instead of inside one of them. This is basically the same as line breaks for code, where one also keeps inner expressions on one line as long as possible and prefers to add line breaks as far towards the root of the AST as possible. (Just no indentations for text.)

@incaseoftrouble
Copy link
Contributor Author

Ok, this makes the diffs very tricky to look at, but I will try :)

@incaseoftrouble
Copy link
Contributor Author

Attempted in #1029

@PhilippWendler
Copy link
Member

Ok, this makes the diffs very tricky to look at, but I will try :)

Hm? The whole idea of this is that it makes diffs easier to read if the lines are not so long (for example on GitHub, which does not provide a word diff).

@incaseoftrouble
Copy link
Contributor Author

Hm? The whole idea of this is that it makes diffs easier to read if the lines are not so long (for example on GitHub, which does not provide a word diff).

Well, one of the many points: If the first word in a sentence / paragraph is changed, this cascades and adds several changed lines / artificial changes.

I have strong opinions on hard wraps in text, but anyway this is moot - the style for benchexec is 80 col wrap so lets follow it :)

@PhilippWendler
Copy link
Member

Hm? The whole idea of this is that it makes diffs easier to read if the lines are not so long (for example on GitHub, which does not provide a word diff).

Well, one of the many points: If the first word in a sentence / paragraph is changed, this cascades and adds several changed lines / artificial changes.

No, don't do this. If the first word in a sentence is changed, then either keep the changed line as is even if it is longer or just break that line. Don't cascade.

@PhilippWendler PhilippWendler changed the title WIP: Improveme Documentation Draft: Improveme Documentation May 21, 2024
@PhilippWendler PhilippWendler marked this pull request as draft May 21, 2024 14:05
@PhilippWendler
Copy link
Member

I guess this can be considered as superseded by #1044 and closed?

@incaseoftrouble
Copy link
Contributor Author

I think there are some points in there but I would have to have a separate look, would definitely end up being a new PR

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

Successfully merging this pull request may close these issues.

Improvements to Documentation
2 participants