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

RST backtick refactor manual.rst #17259

Merged

Conversation

quantimnot
Copy link
Contributor

Added .. default-role:: code directive.

Replaced double backticks with single backticks and realign shifted table columns.

@timotheecour
Copy link
Member

timotheecour commented Mar 5, 2021

@timotheecour
Copy link
Member

@quantimnot how about, for this PR as well as #17258, keep the double backticks when the content contains a \ somewhere inside.

Then we can move forward with this PR without waiting on #17315, and if/when #17315 is merged, we can revisit those small number of cases in subsequent PR.

@@ -360,7 +362,7 @@ contain the following `escape sequences`:idx:\ :
``\\`` `backslash`:idx:
``\"`` `quotation mark`:idx:
``\'`` `apostrophe`:idx:
``\`` '0'..'9'+ `character with decimal value d`:idx:;
``\\`` '0'..'9'+ `character with decimal value d`:idx:;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``\\`` '0'..'9'+ `character with decimal value d`:idx:;
``\`` '0'..'9'+ `character with decimal value d`:idx:;

@@ -471,7 +472,7 @@ literals:
``\\`` `backslash`:idx:
``\"`` `quotation mark`:idx:
``\'`` `apostrophe`:idx:
``\`` '0'..'9'+ `character with decimal value d`:idx:;
``\\`` '0'..'9'+ `character with decimal value d`:idx:;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``\\`` '0'..'9'+ `character with decimal value d`:idx:;
``\`` '0'..'9'+ `character with decimal value d`:idx:;

Comment on lines +6077 to +6083
1. `std`: The `std` pseudo directory is the abstract location of Nim's standard
library. For example, the syntax `import std / strutils` is used to unambiguously
refer to the standard library's `strutils` module.
2. `pkg`: The `pkg` pseudo directory is used to unambiguously refer to a Nimble
package. However, for technical details that lie outside the scope of this document,
its semantics are: *Use the search path to look for module name but ignore the standard
library locations*. In other words, it is the opposite of `std`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. `std`: The `std` pseudo directory is the abstract location of Nim's standard
library. For example, the syntax `import std / strutils` is used to unambiguously
refer to the standard library's `strutils` module.
2. `pkg`: The `pkg` pseudo directory is used to unambiguously refer to a Nimble
package. However, for technical details that lie outside the scope of this document,
its semantics are: *Use the search path to look for module name but ignore the standard
library locations*. In other words, it is the opposite of `std`.
1. `std`: The `std` pseudo directory is the abstract location of Nim's standard
library. For example, the syntax `import std / strutils` is used to unambiguously
refer to the standard library's `strutils` module.
2. `pkg`: The `pkg` pseudo directory is used to unambiguously refer to a Nimble
package. However, for technical details that lie outside the scope of this document,
its semantics are: *Use the search path to look for module name but ignore the standard
library locations*.
In other words, it is the opposite of `std`.

See the `threads <threads.html>`_ and `channels <channels_builtin.html>`_ modules
To enable thread support the `--threads:on` command-line switch needs to
be used. The `system` module then contains several threading primitives.
See the `threads <threads.html>`_ and `channels <channels.html>`_ modules
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
See the `threads <threads.html>`_ and `channels <channels.html>`_ modules
See the `threads <threads.html>`_ and `channels <channels_builtin.html>`_ modules

Copy link
Member

@timotheecour timotheecour left a comment

Choose a reason for hiding this comment

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

LGTM; I've checked thoroughly in 2 different ways:

  • checking the diff
  • checking the rendered html (nim rst2html) and going page by page carefully, comparing the html before PR and after PR; the pages align and any difference would be obvious to the eye; only required 1 place to offset manually (explained below)
  • this PR was rebased but incorrectly dropped the (tiny) diff from 2 recent PR's : rename channels to channels_builtin #17330 and fix RST parsing when no indent after enum.item (fix #17249) #17257 ; this can be fixed up in a followup PR
  • also noted \\ vs \ (ie a \ was inserted wrongly) which can be fixed later
  • everything else was identical, except for things like:
## ``foo``
=>
## `foo`

in code blocks, which render differently (since its verbatim); but this is an expected, desirable change.

@timotheecour timotheecour merged commit 15586c7 into nim-lang:devel Mar 18, 2021
@timotheecour
Copy link
Member

I've merged this after a careful review (see previous comment) to avoid this PR from bit-rotting and needing to be rebased again and again; I've noted the followups needed.

@timotheecour timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Mar 19, 2021
@timotheecour
Copy link
Member

@quantimnot thanks for doing this! could you please followup with a PR to address remaining points? (can be bundled with the fixups needed for #17258 (comment))

@quantimnot quantimnot deleted the manual_rst_backtick_refactor branch March 19, 2021 13:03
ringabout pushed a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: followup needed remove tag once fixed or tracked elsewhere
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants