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

Updateed Documentation to Improve Code Blocks #2727

Merged
merged 4 commits into from
Feb 3, 2024
Merged

Updateed Documentation to Improve Code Blocks #2727

merged 4 commits into from
Feb 3, 2024

Conversation

threeplanetssoftware
Copy link
Contributor

Similar to the PRs to rspec-expectations and rspec-core, this PR just updates the documentation to better provide syntax highlighting for code blocks. Two things I didn't touch which you might want to consider your stance on in the future:

  1. With the indentation of the code snippet, a lot of the code blocks now have extra spaces in front of each line. This isn't Python, so it won't hurt usability, but you may want to normalize those at some point.
  2. The features/GettingStarted.md file has shell commands that were manually preceded with a dollar sign. This is good for indicating it is a shell but bad for copy/pasting successfully. Unlike the above empty white space, that sigil does have a meaning. Because this was already present I did not touch it, but you may want to consider if they should be removed to let someone copy multiple lines at once.

As before, I ran the automated tests and Cucumber didn't report any issues:

178 scenarios (178 passed)
657 steps (657 passed)
8m57.205s

…ated Feature files to identify type of code within code blocks.
Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

Sh/console is optional, but we need to keep the indentation.
I admit I didn’t check tgat in previous PRs.

features/README.md Outdated Show resolved Hide resolved
features/GettingStarted.md Outdated Show resolved Hide resolved
@threeplanetssoftware
Copy link
Contributor Author

Please see the latest push for fixes to the above issues. Will go back and check the other repos I touched.

```ruby
render_template # delegates to assert_template
redirect_to # delegates to assert_redirected_to
```
Copy link
Member

Choose a reason for hiding this comment

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

Features should rename indented with their surrounding text, e.g. RSpec here, they're not markdown and expect indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good evening, thanks for the feedback!

Originally I had these indented at the same level as the surrounding text, however that led to extra spaces at the beginning of the line that @pirj noted. I believe this stems from two aspects of gherkin (and I am not trying to second guess your knowledge of features or Gherkin or anything like that. You all have much greater command of it than I and I'm happy to hear where I'm wrong), which ultimately led to me rendering them as markdown and outdented as far as I did:

  1. The Feature section of Gherkin assumes what is in its description is free-form text until it hits another section heading (such as Scenario in this case). I believe this means that DocStrings aren't recognized as such in that section, so the syntax that is working for Doc Strings later in the Feature won't work up in the description portion.
  2. Doc Strings (scroll down one paragraph from the link) can be either three double-quotes or three backticks. Indentation of the opening quotes or backticks is explicitly unimportant, what matters is indentation of what is inside the quotes. However, due to the caveat above they aren't being recognized in the Feature description portion, so they have to be recognized as Markdown code blocks, if there will be a code block in a Feature's description.

Given the above, I used typical Markdown fenced codeblock syntax, not a Gherkin DocString and outdented the code all the way to the left so that there wouldn't be any extra spaces at the beginning. I could reindent them all one block in and have two extra spaces at the front of each line in the code block and a different solution altogether would be to revert those boxes off of being code boxes and turn them into just block comments. The only reason I went down this path of making them functioning code blocks is that they currently are broken code blocks, so I figured that was the desired path (for example here). Please let me know which of those three solutions you would prefer! And if there's another, better one I haven't considered, please let me know that as well!

The workflow I am using to test these changes is the rspec-dev pipeline (make edits within the repos/rspec-rails folder, run rake update_docs, have rspec.github.io running in LIVERELOAD mode). My hope is it will be close to what you will see when you publish documentation manually.

In closing, the main reason I care to do this is I have been enjoying ya'lls package as I add a bunch of tests to one of my packages, thank you for writing RSpec! I would love to be looking at syntax highlighting in every code example I read as I go through the documentation. I figured if I would take the work to make syntax highlighting work, I might as well contribute it back. I certainly do not want to put the documentation in any state you're not looking for it to be in and I easily could have some of the above mistaken. I would love to learn the right answer!

Have a great evening!

Copy link
Member

Choose a reason for hiding this comment

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

@pirj noted the indentation was wrong in md files, not feature files, only feature files need to be changed back to being indented inline with their body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok! I've pushed the corresponding fix for the feature files. Have a great evening!

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

I'm happy to merge this when the feature files are re-indented, but someone needs to go and check all these fence changes against the website generation and not just githubs rendering

before(:context) do
@widget = Widget.create!
end
```ruby
Copy link
Member

Choose a reason for hiding this comment

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

I think we should indent this, otherwise the numbered list looks weird.

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

Looks great. A minor nitpick regarding the numbered list. I’m happy with this change either eay.

@threeplanetssoftware
Copy link
Contributor Author

My apologies for not noticing the ordered list repeating number 1. I have fixed the indentation of those blocks so that the list is ordered again. I ran the cucumber checks again and it is still passing. Have a great evening,

@pirj
Copy link
Member

pirj commented Feb 3, 2024

check all these fence changes against the website generation

Two observations:

  • we didn’t publish rspec-rails 6.1 docs. There’s not much besides send_email matcher, though
  • middleman build spits some errors

I will take a closer look later.

@JonRowe JonRowe merged commit 06d05ab into rspec:main Feb 3, 2024
17 checks passed
@threeplanetssoftware
Copy link
Contributor Author

Thank you!

@threeplanetssoftware threeplanetssoftware deleted the update-documentation-for-code-blocks branch February 3, 2024 14:13
@JonRowe
Copy link
Member

JonRowe commented Feb 4, 2024

These have been published, they don't change the output though because our syntax highlighting on features at least don't support the extra information I guess

@threeplanetssoftware
Copy link
Contributor Author

Good morning! I appreciate your pushing a fresh copy of the docs live, I know that can be tedious.

I believe the updated formatting should work, as I was able to use your rspec-dev and rspec.github.io repositories to view it. I suspect the issue is that the latest rspec-rails tag (6.1.1) was cut two weeks ago, prior to merging these documentation changes.

I can recreate the formatting issue on the current rspec-dev repository by running rake update-docs["6.1.1"]. If I add a new tag to rspec-rails of 6.1.2 at the current head, then update_docs displays the newer formatting.

image

Note: I do not expect you to cut a new release just for documentation, but whenever you do have a new release, I believe this will work. Thank you again for your work on rspec, I have been adding a lot of it in the past week and have greatly enjoyed it.

@JonRowe
Copy link
Member

JonRowe commented Feb 5, 2024

The build ran against 6-1-maintenance which is supposed to pull in changes even if not released, I also noticed no change locally on my machine 🤔

JonRowe added a commit that referenced this pull request Feb 23, 2024
…on-for-code-blocks

Improve code blocks in documentation
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