Reduce code duplication in about and error pages

[Ryan J Ollos]

Having to make modifications in both error.html and about.html in #11558, I was looking at the duplicated code and possibilities for re-use.

The major change that will be proposed is the extraction of code to a new template environment_info.html, which is included in both error.html and about.html.

The differences I could see between the information displayed on the two pages and the steps I tool to resolve the differences are:

1. Headings on tables in about.html, but no headings in error.html - environment_info.html has heading on tables.
2. Plugins table on about.html also shows disabled plugins - existing behavior preserved on both pages.
3. Plugins table on error.html links to frames in stacktrace - existing behavior preserved on both pages.
4. Markup was slightly different, with divs enclosing each table on about.html, and the ids on the divs rather than on the tables, as error.html does - tables in environment_info.html now have enclosing divs.
5. The plugins section on the error page read Enabled Plugins; on the about page it read Installed Plugins. The environment_info.html template reads Installed Plugins, however the error.html page continues to show only enabled plugins, whereas the about.html page shows both Enabled and Disabled plugins.

The change in markup on error.html in (4) means that these changes should only be made on the trunk. These changes will also help with #11548, in reducing the number of code changes needed for adding templates to the environment info on about.html and error.html (at least on the trunk).

The CSS for #content.error and #content.about was very similar. I merged about.css into trac.css, and merged several of the rules so that they apply to the new template environment_info.html.

[Ryan J Ollos]

I'll wait until the changes in #11558 are committed and rebase my local t11565 branch on those changes before posting log:rjollos.git:t11565.

[Ryan J Ollos]

When the about.py module was added in [4264] there was a page for each section: /about/config and /about/plugins. In [4593] the sections were combined into a single page. The regex could be simplified after [4593]:

-re.match(r'/about(?:_trac)?(?:/.+)?$', req.path_info)
+re.match(r'/about(?:_trac)?$', req.path_info)

Is there any reason to not make this change on the trunk?

[Ryan J Ollos]

Proposed changes can be found in log:rjollos.git:t11565.

[Ryan J Ollos]

Change from comment:4 committed to trunk in [12649].

[Ryan J Ollos]

Refactoring of about.html and error.html committed in [12650,12652].

[Jun Omae]

Sorry for late.

I don't think that we should add system_info decorated with @property because Environment class has systeminfo property. That's confusing.

[Ryan J Ollos]

I didn't notice. It's as confusing as having get_systeminfo and get_system_info. systeminfo could be changed to _systeminfo.

[Ryan J Ollos]

Replying to rjollos:

systeminfo could be changed to _systeminfo.

Nevermind, that doesn't make sense. I'll look more closely.

[Jun Omae]

Nevermind, that doesn't make sense. I'll look more closely.

Okay.

Also, I noticed that Trac core and some plugins in trac-hacks directly append entries to env.systeminfo….

$ grep -rF '.systeminfo' trac tracopt
trac/test.py:        self.systeminfo = []
trac/env.py:        self.systeminfo = []
trac/env.py:        info = self.systeminfo[:]
trac/db/mysql_backend.py:            self.env.systeminfo.extend([('MySQL', mysql_info),
trac/db/sqlite_backend.py:            self.env.systeminfo.extend([('SQLite', sqlite_version_string),
trac/db/postgres_backend.py:            self.env.systeminfo.append(('psycopg2', self._version))
trac/tests/env.py:        self.systeminfo = []
trac/web/main.py:                env.systeminfo.append((env.webfrontend,
tracopt/versioncontrol/git/git_fs.py:            self.env.systeminfo.append(('GIT', self._version['v_str']))

$ grep -rF --include='*.py' '.systeminfo' .
./fullblogplugin/0.11/tracfullblog/core.py:        self.env.systeminfo.append(('FullBlog',
./customfieldadminplugin/0.11/customfieldadmin/tests/api.py:                    in self.env.systeminfo)
./customfieldadminplugin/0.11/customfieldadmin/api.py:            self.env.systeminfo.append(('CustomFieldAdmin', __version__))
./xmlrpcplugin/trunk/tracrpc/api.py:        self.env.systeminfo.append(('RPC',
./gitplugin/0.11/tracext/git/git_fs.py:                        self.env.systeminfo.append(('GIT', self._version['v_str']))
./gitplugin/0.12/tracext/git/git_fs.py:                        self.env.systeminfo.append(('GIT', self._version['v_str']))
./bittenforgitplugin/0.11/0.6b2/bitten/master.py:        self.env.systeminfo.append(('Bitten',

[Ryan J Ollos]

Python 2.5 support has been officially dropped on the trunk: TracDev/ApiChanges/1.1.1, so we can use the ​setter attribute.

These changes might work, but I'll need to do more testing tomorrow: log:rjollos.git:t11565.3. After the change accessing Environment.systeminfo will return a list with more entries than previously. I'm not sure there's a perfect solution that leave the API completely unchanged, but it doesn't look like anyone is doing a get on systeminfo, so it seems like a fairly safe change. The other solution, a partial revert of [12650] leaves us with the confusing get_systeminfo and get_system_info, which is not good.

[Ryan J Ollos]

Addition of systeminfo property reverted in [12653].

[Ryan J Ollos]

Replying to jomae:

Also, I noticed that Trac core and some plugins in trac-hacks directly append entries to env.systeminfo….

It seems like it would be cleaner if we used ISystemInfoProvider everywhere. Care would need to be taken for issues like that noted in comment:5:ticket:8908. I might explore whether some refactoring is possible.

[Ryan J Ollos]

Replying to rjollos:

It seems like it would be cleaner if we used ISystemInfoProvider everywhere. Care would need to be taken for issues like that noted in comment:5:ticket:8908. I might explore whether some refactoring is possible.

These changes were made in #11587.

[Ryan J Ollos]

Related changes proposed in comment:34:ticket:11901.