Sentence checker

[DLu]

And now, a reading from the book of Code Style

Each sentence must start on a new line.

@clalancette is a real stickler for this, so I made a linter. Unfortunately, there are lots of problems, some of which are the linter, and some of which are the content.

Happy to have help. I can't devote a ton more time to this, but I still wanted the code to be out there.

[DLu]

Very important note: I resisted the urge to call this "Edgar Jr."

https://www.youtube.com/watch?v=NW14YWw3h1E

[fujitatomoya]

@DLu

Unfortunately, there are lots of problems, some of which are the linter, and some of which are the content.

i just got interested in this. can you rephrase this? are you saying there are many false alarms?

[DLu]

Taking a look at the current results on my machine, it seems like mostly the output is true positives, with a handful of false positives mixed in. The false positives include:

• Numbered lists
• Day abbreviations
• People who use punctuation in their given names. Particularly ironic, coming from me, don't you think @nuclearsandwich ?

Current error count for the whole repo: 624

[fujitatomoya]

@DLu sounds good, if there is anything i can help, please let me know. i will take a look at the PR.

[DLu]

Down to 285

[fujitatomoya]

ninja quick 🥷 amazing 💯

[cottsay]

I haven't dug into this, but it might be good to actually parse the RST so we don't need to worry about handling the syntax. Example of where we're already parsing RST: https://github.com/ros-infrastructure/catkin_pkg/blob/master/src/catkin_pkg/changelog.py

[fujitatomoya]

@Mergifyio rebase

[mergify[bot]]

rebase

☑️ Nothing to do

• [ ] -conflict [📌 rebase requirement]
• [X] -closed [📌 rebase requirement]
• [X] queue-position = -1 [📌 rebase requirement]
• [X] any of:
  • [X] #commits > 1 [📌 rebase requirement]
  • [X] #commits-behind > 0 [📌 rebase requirement]
  • [X] -linear-history [📌 rebase requirement]

[fujitatomoya]

@DLu can you rebase this against rolling?

[DLu]

Note: This PR has been split up.

• The functionality is going in this repo: https://github.com/DLu/sphinx_tamer
• The content fixes are going in other PRs, such as #5001 #5017 #5030 and #5031 (and possibly more in the future)

[DLu]

@fujitatomoya My sentence checker is now checked in here: https://github.com/DLu/sphinx_tamer

At some point, I could reintegrate it with this PR, or we can work to integrate it into a GitHub check. Which do you think makes the most sense?

[fujitatomoya]

@DLu sorry for being late back to you, i was on the vacation last week.

i am trying to catch up with you.

At some point, I could reintegrate it with this PR, or we can work to integrate it into a GitHub check.

these are different implementation for me, can be in the same PR though.

what i am not really sure is,

since sphinx_tamer isn't available on the Python Package Index (PyPI), we'll need to install it directly from its GitHub repository, right? and the call it from here as sphinx extension via make test or another make target? once this is clear, i think having it on github action is pretty straight forward. could you enlighten me this design a bit?

[DLu]

Worth noting that this new version with sphinx_sentence_scan_single is a semi-hacky backup. I intended to run the Python directly instead of using subprocess, but Sphinx is a bit particular about how it runs as a library.

Here is the "clean" direct-Python version of the checker

However, this interacts with sphinx_sitemap_ros.py in a way that is currently beyond my understanding of Sphinx.

! ros2_documentation/ > make lint
./sphinx-lint-with-ros source
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/sphinx/events.py", line 98, in emit
    results.append(listener.handler(self.app, *args))
  File "/home/dlu/OpenSource/ros2_documentation/plugins/sphinx_sitemap_ros.py", line 112, in record_builder_type
    builder.env.app.sitemap_links = Manager().Queue()
  File "/usr/lib/python3.10/multiprocessing/context.py", line 57, in Manager
    m.start()
  File "/usr/lib/python3.10/multiprocessing/managers.py", line 562, in start
    self._process.start()
  File "/usr/lib/python3.10/multiprocessing/process.py", line 118, in start
    assert not _current_process._config.get('daemon'), \
AssertionError: daemonic processes are not allowed to have children

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/local/lib/python3.10/dist-packages/sphinxlint/cli.py", line 174, in _check_file
    return check_file(*todo)
  File "/usr/local/lib/python3.10/dist-packages/sphinxlint/sphinxlint.py", line 66, in check_file
    return check_text(filename, text, checkers, options)
  File "/usr/local/lib/python3.10/dist-packages/sphinxlint/sphinxlint.py", line 45, in check_text
    for lno, msg in check(
  File "/home/dlu/OpenSource/ros2_documentation/plugins/ros_checkers.py", line 34, in check_sentence_count
    settings, reporter, parser = load_sphinx(
  File "/home/dlu/Projects/sphinx_tamer/src/sphinx_tamer/sphinx_wrapper.py", line 47, in load_sphinx
    app = Sphinx(srcdir=src_path,
  File "/usr/local/lib/python3.10/dist-packages/sphinx/application.py", line 292, in __init__
    self._init_builder()
  File "/usr/local/lib/python3.10/dist-packages/sphinx/application.py", line 366, in _init_builder
    self.events.emit('builder-inited')
  File "/usr/local/lib/python3.10/dist-packages/sphinx/events.py", line 109, in emit
    raise ExtensionError(
sphinx.errors.ExtensionError: Handler <function record_builder_type at 0x7426fd50d900> for event 'builder-inited' threw an exception (exception: daemonic processes are not allowed to have children)
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/dlu/OpenSource/ros2_documentation/./sphinx-lint-with-ros", line 7, in <module>
    main(sys.argv)
  File "/usr/local/lib/python3.10/dist-packages/sphinxlint/cli.py", line 238, in main
    count = print_errors(
  File "/usr/local/lib/python3.10/dist-packages/sphinxlint/cli.py", line 199, in print_errors
    for error in errors:
  File "/usr/local/lib/python3.10/dist-packages/sphinxlint/cli.py", line 180, in sort_errors
    for results in results:
  File "/usr/lib/python3.10/multiprocessing/pool.py", line 873, in next
    raise value
sphinx.errors.ExtensionError: Handler <function record_builder_type at 0x7426fd50d900> for event 'builder-inited' threw an exception (exception: daemonic processes are not allowed to have children)
make: *** [Makefile:26: lint] Error 1

If the sitemap file is not there, it does not have this problem.

[DLu]

Didn't realize I left this in draft state. Conflicts are resolved, ready for review.

[christophebedard]

We intend to backport this and fix the merge conflicts + linter issues, right?

[fujitatomoya]

MERGING 🚀🚀🚀🚀🚀 @DLu thanks for amazing work 👍