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]

rebase

☑️ Nothing to do

Details

• -conflict [📌 rebase requirement]
• -closed [📌 rebase requirement]
• queue-position = -1 [📌 rebase requirement]
• any of:
  • #commits > 1 [📌 rebase requirement]
  • #commits-behind > 0 [📌 rebase requirement]
  • -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 Clean up sentences (white space changes only) #5001 Clean up sentences II (white space changes only) #5017 Fix smart quotes #5030 and Other sentence formatting #5031 (and possibly more in the future)

[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.

[sloretz]

LGTM! I'll ask for one more review since I made a few small changes.

[christophebedard]

Minor comments.

[nuclearsandwich]

It's a huge level-up to have this as an automated check because this style requirement is motivated by making change review using textual diff tools much more natural.

Helping contributors check the constraint will save their time and maintainers'!

[christophebedard]

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

[fujitatomoya]

a couple of comments.

[fujitatomoya]

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