sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

#8904: value.split(';', n - 1) -> value.split('; ', n - 1)

Open KaKkouo opened this issue 4 years ago • 5 comments

Subject: #8904 fix about split_into function.

Feature or Bugfix

  • Bugfix

Purpose

  • For value.split used in the split_into function, the argument has been modified to properly separate the terms.
  • Since this is a modification of the term delimiters in the index directive, there is no particular environment or extension that it depends on.

Detail

about syntax index directive

  • A semicolon is followed by a space.
  • Therefore, changing the argument of the value.split function from ";" to "; "+space is no problem. In fact, it is appropriate to change it.
.. index:: term1; term2

source code

  • sphinx/util/init.py
    • The target of this revision.
  • sphinx/environment/adapters/indexentries.py
  • IndexEntries class, create_index method
    • split_into is being used.
    • The fix was written in #8904.
  • sphinx/writers/latex.py
    • LaTeXTranslator class, visit_index method
      • split_into is being used.
      • The fix for split_into should not be a problem since it is a read-dependent change, but I have not checked it.

Relates

  • #8904

KaKkouo avatar Oct 18 '21 09:10 KaKkouo

  • I've seen the circleci FAIL, and I know from test_html_index_entries that it's a test related to split_into, but I can't figure out why I'm getting the "MAILING LIST" error.
  • This situation is not explained in the Contributing to Sphinx section, should I just wait?

KaKkouo avatar Oct 18 '21 09:10 KaKkouo

Thank you for your contribution.

At present, term1;term2 (without whitespace) is recognized as two parts. In other words, it's a valid definition. Therefore, this fix brings a breaking change too.

tk0miya avatar Oct 20 '21 17:10 tk0miya

It turned out that the proposed changes were the ones that would break compatibility. Thanks for letting me know. Also, I apologize for the hassle.

KaKkouo avatar Oct 23 '21 02:10 KaKkouo

I would like to discuss how to handle #8904. Should I just close it with "not a bug"? Or should I leave it to the Sphinx members?

If it is better for me to handle it, I will explain the following points:

  • It's not a bug.
  • He can fix the code or create a Sphinx extension to use it in his project.
  • Or, he have to create sphinx/domains/prolog.py

KaKkouo avatar Oct 23 '21 02:10 KaKkouo

Marking this as an "enhancement" -- the way forward here would be to detect ; (no space) and raise a warning noting that in Sphinx 7.0 we would change to require ; (with a space). Then the prolog use-case would work.

A complementary approach would be an opt-in configuration variable to require ; (with space) now, allowing the prolog use-case today rather than when Sphinx 7.0 is released.

A

AA-Turner avatar May 23 '22 02:05 AA-Turner