Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15992 closed Cleanup/optimization (fixed)

Reference settings properly

Reported by: aaugustin Owned by: nobody
Component: Documentation Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX:

Description

According to http://docs.djangoproject.com/en/dev/internals/documentation/ settings should be marked up like this:

:setting:`ADMIN_FOR`

Attached patch implements this throughout the documentation, except a few instances in the internals/documentation.txt page where this element of the markup is explained.

Attachments (4)

15992.patch (60.7 KB) - added by aaugustin 3 years ago.
15992.2.patch (56.7 KB) - added by aaugustin 3 years ago.
15992.4.patch (54.2 KB) - added by aaugustin 3 years ago.
15992.5.patch (52.7 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (13)

Changed 3 years ago by aaugustin

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

This patch is not perfect, but it's still a good step forward, so I submitted it:

  • Lines are not rewrapped to 80 chars; however, this is not strictly enforced in the documentation, and re-indenting entire paragraphs would make the diff completely unreadable.
  • settings that have sub-settings (DATABASES, CACHES, etc.) may not be completely fixed since they use a different syntax: :setting:`BACKEND <CACHES-BACKEND>`

For those who wonder, yes, it was automatically generated, here is the script:

#!/usr/bin/env python

import fileinput
import re
import sys

setting_re = re.compile(r'\.\. setting:: ([A-Z0-9_]+)')

settings = set()

for line in fileinput.input():
    match = setting_re.match(line)
    if match:
        settings.add(match.group(1))

for f in sys.argv[1:]:
    modified = False
    with open(f, 'r') as h:
        content = h.read()
        for setting in settings:
            if '``%s``' % setting in content:
                content = content.replace('``%s``' % setting, ':setting:`%s`' % setting)
                modified = True
    if modified:
        with open(f, 'w+') as h:
            h.write(content)

comment:2 Changed 3 years ago by aaugustin

  • Patch needs improvement set

As one could expect, there are a few false positives, for instance in docs/topics/i18n/internationalization.txt. A manual review is necessary, I'll post a new patch soon.

comment:3 Changed 3 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Accepted

Changed 3 years ago by aaugustin

comment:4 Changed 3 years ago by aaugustin

I proof read the patch and removed the following false positives:

  • DEBUG as a log level and not as a setting
  • context variables for templates having the same name as a setting

comment:5 Changed 3 years ago by aaugustin

  • Patch needs improvement unset

Here is a command to check the strings that could be missed by the patch:

find . -name '*.txt' -print0 | xargs -0 egrep -C 5 '``(ABSOLUTE_URL_OVERRIDES|ADMINS|ADMIN_FOR|ADMIN_MEDIA_PREFIX|ALLOWED_INCLUDE_ROOTS|APPEND_SLASH|AUTHENTICATION_BACKENDS|AUTH_PROFILE_MODULE|CACHES|CACHE_BACKEND|CACHE_MIDDLEWARE_ALIAS|CACHE_MIDDLEWARE_ANONYMOUS_ONLY|CACHE_MIDDLEWARE_KEY_PREFIX|CACHE_MIDDLEWARE_SECONDS|COMMENTS_APP|COMMENTS_HIDE_REMOVED|COMMENT_MAX_LENGTH|CSRF_COOKIE_DOMAIN|CSRF_COOKIE_NAME|CSRF_COOKIE_PATH|CSRF_COOKIE_SECURE|CSRF_FAILURE_VIEW|DATABASE|DATABASES|DATABASE_ENGINE|DATABASE_HOST|DATABASE_NAME|DATABASE_OPTIONS|DATABASE_PASSWORD|DATABASE_PORT|DATABASE_ROUTERS|DATABASE_USER|DATETIME_FORMAT|DATETIME_INPUT_FORMATS|DATE_FORMAT|DATE_INPUT_FORMATS|DEBUG|DECIMAL_SEPARATOR|DEFAULT_CHARSET|DEFAULT_CONTENT_TYPE|DEFAULT_FILE_STORAGE|DEFAULT_FROM_EMAIL|DEFAULT_INDEX_TABLESPACE|DEFAULT_TABLESPACE|DISALLOWED_USER_AGENTS|EMAIL_BACKEND|EMAIL_FILE_PATH|EMAIL_HOST|EMAIL_HOST_PASSWORD|EMAIL_HOST_USER|EMAIL_PORT|EMAIL_SUBJECT_PREFIX|EMAIL_USE_TLS|FILE_CHARSET|FILE_UPLOAD_HANDLERS|FILE_UPLOAD_MAX_MEMORY_SIZE|FILE_UPLOAD_PERMISSIONS|FILE_UPLOAD_TEMP_DIR|FIRST_DAY_OF_WEEK|FIXTURE_DIRS|FORMAT_MODULE_PATH|GDAL_LIBRARY_PATH|GEOIP_CITY|GEOIP_COUNTRY|GEOIP_LIBRARY_PATH|GEOIP_PATH|GEOS_LIBRARY_PATH|HOST|IGNORABLE_404_ENDS|IGNORABLE_404_STARTS|IGNORABLE_404_URLS|INSTALLED_APPS|INTERNAL_IPS|LANGUAGES|LANGUAGE_CODE|LANGUAGE_COOKIE_NAME|LOCALE_PATHS|LOGGING|LOGGING_CONFIG|LOGIN_REDIRECT_URL|LOGIN_URL|LOGOUT_URL|MANAGERS|MEDIA_ROOT|MEDIA_URL|MIDDLEWARE_CLASSES|MONTH_DAY_FORMAT|NAME|NUMBER_GROUPING|OPTIONS|PASSWORD|PASSWORD_RESET_TIMEOUT_DAYS|PORT|POSTGIS_TEMPLATE|POSTGIS_VERSION|PREPEND_WWW|PROFANITIES_LIST|RESTRUCTUREDTEXT_FILTER_SETTINGS|ROOT_URLCONF|SECRET_KEY|SEND_BROKEN_LINK_EMAILS|SERIALIZATION_MODULES|SERVER_EMAIL|SESSION_COOKIE_AGE|SESSION_COOKIE_DOMAIN|SESSION_COOKIE_HTTPONLY|SESSION_COOKIE_NAME|SESSION_COOKIE_PATH|SESSION_COOKIE_SECURE|SESSION_ENGINE|SESSION_EXPIRE_AT_BROWSER_CLOSE|SESSION_FILE_PATH|SESSION_SAVE_EVERY_REQUEST|SHORT_DATETIME_FORMAT|SHORT_DATE_FORMAT|SIGNING_BACKEND|SITE_ID|SPATIALITE_SQL|STATICFILES_DIRS|STATICFILES_FINDERS|STATICFILES_STORAGE|STATIC_ROOT|STATIC_URL|TEMPLATE_CONTEXT_PROCESSORS|TEMPLATE_DEBUG|TEMPLATE_DIRS|TEMPLATE_LOADERS|TEMPLATE_STRING_IF_INVALID|TEST_CHARSET|TEST_COLLATION|TEST_DATABASE_CHARSET|TEST_DATABASE_COLLATION|TEST_DATABASE_NAME|TEST_DEPENDENCIES|TEST_MIRROR|TEST_NAME|TEST_RUNNER|TEST_USER|THOUSAND_SEPARATOR|TIME_FORMAT|TIME_INPUT_FORMATS|TIME_ZONE|URL_VALIDATOR_USER_AGENT|USER|USE_ETAGS|USE_I18N|USE_L10N|USE_THOUSAND_SEPARATOR|YEAR_MONTH_FORMAT)``'

Currently, after applying the patch, all the lines extracted by this command are false positives.

Changed 3 years ago by aaugustin

comment:6 Changed 3 years ago by jezdez

  • Patch needs improvement set

Search and replacing the settings names isn't sensible, e.g. the section headlines don't need :setting: statements.

comment:7 Changed 3 years ago by aaugustin

This problem occurred twice in docs/ref/contrib/gis/install.txt and three times in docs/ref/contrib/gis/testing.txt, nowhere else. New patch fixes this.

Changed 3 years ago by aaugustin

comment:8 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

In [16290]:

Fixed #15992 -- Added more references to settings. Thanks, aaugustin.

comment:9 Changed 3 years ago by jezdez

In [16291]:

[1.3.X] Fixed #15992 -- Added more references to settings. Thanks, aaugustin.

Backport from trunk (r16290).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.