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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-106948: Add Doc/nitpick_ignore.yml config file #107278

Closed
wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jul 25, 2023

Maintaining nitpick_ignore Python list in conf.py is a burden. Add a YAML configuration file which is easier to maintain.


馃摎 Documentation preview 馃摎: https://cpython-previews--107278.org.readthedocs.build/

@vstinner
Copy link
Member Author

I prefer YAML over TOML to write a list of strings with bare minimum formating: no quote, no [..., ..., ...]```, just a bullet point - ``.

@vstinner
Copy link
Member Author

Maintaining nitpick_ignore Python list in conf.py is a burden. Add a
YAML configuration file which is easier to maintain.

Ignore also standard C types in the "c:identifier" domain.

Update Doc/requirements-oldest-sphinx.txt.
@AA-Turner
Copy link
Member

Cross-posting from discord: I鈥檇 prefer not to create an additional (CPython-custom) YAML file here if possible, perhaps instead we could have a nitpick_ignore.py that we just import into conf.py? I assume the objection to the variable being within conf.py is that the length makes the file unwieldy.

A

@vstinner
Copy link
Member Author

I assume the objection to the variable being within conf.py is that the length makes the file unwieldy.

I would like to add a configuration file since writing a long list of strings in Python if annoying. The YAML file is less verbose.

Also, Standard C types (types section) is duplicated: used in the c:type domain and in the c:identifier domain.

Compare:

    ('c:func', 'calloc'),
    ('c:func', 'dlopen'),
    ('c:func', 'exec'),
    ('c:func', 'fcntl'),
    ('c:func', 'fork'),
    ('c:func', 'free'),
    ('c:func', 'gmtime'),

to:

  - calloc
  - dlopen
  - exec
  - fcntl
  - fork
  - free
  - gmtime

@vstinner
Copy link
Member Author

Hum, shown diferently, I have a local branch to complete these nitpick ignore list. I added many functions, macros, etc. and the list is now quite big:

nitpick_ignore = [
    # Standard C macros
    ('c:macro', 'EDOM'),
    ('c:macro', 'EINTR'),
    ('c:macro', 'LLONG_MAX'),
    ('c:macro', 'LLONG_MIN'),
    ('c:macro', 'LONG_MAX'),
    ('c:macro', 'LONG_MIN'),
    ('c:macro', 'SIGINT'),
    # Standard C variables
    ('c:data', 'errno'),
    # Standard environment variables
    ('envvar', 'BROWSER'),
    ('envvar', 'COLUMNS'),
    ('envvar', 'COMSPEC'),
    ('envvar', 'DISPLAY'),
    ('envvar', 'HOME'),
    ('envvar', 'HOMEDRIVE'),
    ('envvar', 'HOMEPATH'),
    ('envvar', 'IDLESTARTUP'),
    ('envvar', 'LANG'),
    ('envvar', 'LANGUAGE'),
    ('envvar', 'LC_ALL'),
    ('envvar', 'LC_CTYPE'),
    ('envvar', 'LC_COLLATE'),
    ('envvar', 'LC_MESSAGES'),
    ('envvar', 'LC_MONETARY'),
    ('envvar', 'LC_NUMERIC'),
    ('envvar', 'LC_TIME'),
    ('envvar', 'LINES'),
    ('envvar', 'LOGNAME'),
    ('envvar', 'PAGER'),
    ('envvar', 'PATH'),
    ('envvar', 'PATHEXT'),
    ('envvar', 'SOURCE_DATE_EPOCH'),
    ('envvar', 'TEMP'),
    ('envvar', 'TERM'),
    ('envvar', 'TMP'),
    ('envvar', 'TMPDIR'),
    ('envvar', 'TZ'),
    ('envvar', 'USER'),
    ('envvar', 'USERNAME'),
    ('envvar', 'USERPROFILE'),
    # Win32 API
    ('c:func', 'FormatMessage'),
    ('c:func', 'GetFileInformationByHandle'),
    ('c:func', 'GetLastError'),
    ('c:func', 'GetVersionEx'),
    ('c:func', 'MessageBeep'),
    ('c:func', 'PlaySound'),
    ('c:func', 'ShellExecute'),
    ('c:func', 'TerminateProcess'),
    ('c:func', 'VirtualAlloc'),
    ('c:func', 'VirtualFree'),
    ('c:func', 'WSAIoctl'),
    # Win32 macros
    ('c:macro', 'CP_ACP'),
    # Do not error nit-picky mode builds when _SubParsersAction.add_parser cannot
    # be resolved, as the method is currently undocumented. For context, see
    # https://github.com/python/cpython/pull/103289.
    ('py:meth', '_SubParsersAction.add_parser'),
]

# Standard C types
c_std_types = """
FILE
__int64
int64_t
intmax_t
off_t
ptrdiff_t
siginfo_t
size_t
ssize_t
time_t
uint64_t
uintmax_t
uintptr_t
va_list
wchar
wchar_t
""".strip().split()

for name in c_std_types:
    nitpick_ignore.append(('c:type', name))
    # "c:identifier" domain is used by types in ".. c:function:" definitions
    nitpick_ignore.append(('c:identifier', name))

# Standard C functions
c_std_funcs = """
_exit
_stricmp
abort
atof
calloc
close
ctime
dlopen
exec
exit
fcntl
flock
fork
free
fstat
fsync
gettimeofday
getsid
gmtime
inet_aton
inet_pton
ioctl
localeconv
localtime
lockf
lstat
main
malloc
memmove
memcpy
mktime
mmap
munmap
open
perror
posix_spawn
posix_spawn_file_actions_addclose
posix_spawn_file_actions_adddup2
posix_spawn_file_actions_addopen
posix_spawnp
printf
putenv
qsort
realloc
select
setenv
setpgid
setpgrp
setsid
setsockopt
sigaction
sigaltstack
siginterrupt
signal
snprintf
splice
sprintf
stat
statvfs
strcasecmp
strcmp
strerror
strlen
strncmp
system
unsetenv
vsnprintf
vsprintf
""".strip().split()
for name in c_std_funcs:
    nitpick_ignore.append(('c:func', name))

Note: this list is incomplete :-)

@vstinner
Copy link
Member Author

This PR fix warnigs like:

c:identifier reference target not found: va_list

I get this warning more and more on PR changing the C API. Example in a PR changing the C API doc: https://github.com/python/cpython/pull/107274/files

Even if va_list is already in Doc/conf.py, the c:function markup looks for types in c:identifier and so emit a warning that the type is unknown. This PR adds va_list to c:type and c:identifier domains.

@hugovk
Copy link
Member

hugovk commented Jul 26, 2023

I've a slight (non-blocking) preference to avoid adding more code that needs maintaining, and having Sphinx deal with reading config.

Using standard Sphinx things also makes things more familiar for new contributors. And we're also cautious about adding new dependencies (PyYAML) in case downstream redistributors haven't installed them, for example, see sphinxext.opengraph.

I don't mind the existing nitpick_ignore list too much. Moving it to another Python file is okay too.

But it's more important to me to improve the docs, including fixing/ignoring warnings, so if others also like this approach, it's fine by me too.

@serhiy-storchaka
Copy link
Member

It is a shame that Spjinx requires duplicating c:type names as c:identifier.

It can be fixed by a simple code:

for role, name in nitpick_ignore:
    if role == 'c:type':
        nitpick_ignore.append(('c:identifier', name))
del name, role

Would not it enough?

For now there is no significant difference between Python and YAML. Inany case we will add only few lines at once and not going to grow the nitpick_ignore list too much. Too much names here increase a chance of using incorrect role with a name from this list (it already happened with LINES and COLS).

@vstinner
Copy link
Member Author

It is a shame that Spjinx requires duplicating c:type names as c:identifier. It can be fixed by a simple code: (...) Would not it enough?

I wrote PR #107295 to implement exactly that (I made minor changes in your proposed code).

@vstinner
Copy link
Member Author

I abandon this PR in favor of PR #107295 (merged) and PR #107301. Thanks for your feedback.

@vstinner vstinner closed this Jul 26, 2023
@vstinner vstinner deleted the nitpick_ignore branch July 26, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants