Context Navigation

#12787 closed task (fixed)

Remove deprecated code (Trac 1.5.1) — at Version 26

Reported by:	Ryan J Ollos	Owned by:	Ryan J Ollos
Priority:	normal	Milestone:	1.5.1
Component:	general	Version:
Severity:	normal	Keywords:
Cc:		Branch:
Release Notes:
API Changes:	Removed deprecated code: Genshi template framework `InvalidAttachment` and `InvalidTicket` exceptions Accessor in `TicketModule` for moved options: `max_comment_size`, `max_description_size`, `max_summary_size` (moved to `TicketSystem`). `Environment.get_systeminfo` `EnvironmentStub.clear_component_registry` and `EnvironmentStub.restore_component_registry` `IAttachmentChangeListener.attachment_reparented` and `Attachment.reparent` Deprecated parameters of `Query` methods `Request.send_error` no longer renders an error page `charset` and `ie_if` attributes from script elements (and from function `add_script`) `excanvas.js`
Internal Changes:

Description

Like #11901 for 1.3.1, deprecated code scheduled for removal in 1.5.1 will be listed in this ticket.

Change History (26)

follow-up: 3 comment:1 by Ryan J Ollos, 7 years ago

Is the intention that plugins in 1.5.1 and later should always implement the method signature post_process_request(self, req, template, data, metadata=None), or is def post_process_request(req, template, data) also allowed as a method signature for a class implementing IRequestFilter?

comment:2 by Ryan J Ollos, 7 years ago

Changes in log:rjollos.git:t12787_remove_deprecated_code. It will be quite a while before the changes are committed, but they could use some review. All tests pass for me, so if there are issues then it will be an opportunity to improve test coverage.

There's one TODO in chrome.py: Chrome.render_template .

I'll rebase and post revised changes periodically.

Last edited 7 years ago by Ryan J Ollos (previous) (diff)

in reply to: 1 comment:3 by Ryan J Ollos, 7 years ago

Replying to Ryan J Ollos:

Is the intention that plugins in 1.5.1 and later should always implement the method signature post_process_request(self, req, template, data, metadata=None), or is def post_process_request(req, template, data) also allowed as a method signature for a class implementing IRequestFilter?

I think I overlooked that metadata=None was for backward-compatibility, and in the 1.5.1+ API it should be a positional, not a keyword, argument. Therefore the correct signature is post_process_request(self, req, template, data, metadata).

Additional changes in [ff9c6753/rjollos.git]. For the case in which the process_request does not return metadata, an empty dict is passed to the post_process_request methods.

comment:4 by Ryan J Ollos, 7 years ago

Rebased and additional changes in log:rjollos.git:t12787_remove_deprecated_code.1.

comment:5 by Ryan J Ollos, 7 years ago

Scheduled EnvironmentStub.clear_component_registry and EnvironmentStub.restore_component_registry for removal in 1.5.1: r15980.

comment:6 by Ryan J Ollos, 7 years ago

Milestone:	next-major-releases → 1.5.1

comment:7 by Ryan J Ollos, 7 years ago

Rebased changes in log:rjollos.git:t12787_remove_deprecated_code.2.

follow-up: 9 comment:8 by Ryan J Ollos, 7 years ago

Rebased changes in log:rjollos.git:t12787_remove_deprecated_code.3. Tests pass with and without Genshi.

Is the following needed when support for Genshi is removed?: trunk/trac/util/html.py@16037:712-713#L698 .

Should we allow the metadata argument of process_request to be None with removal of Genshi support? From TracDev/PortingFromGenshiToJinja#IRequestHandler:

Note that as long as we have to support the legacy Genshi templates, a None value passed as third argument won't be interpreted as an empty dict, but rather as an empty content_type.

in reply to: 8 ; follow-up: 10 comment:9 by Christian Boos, 7 years ago

Replying to Ryan J Ollos:

Rebased changes in log:rjollos.git:t12787_remove_deprecated_code.3. Tests pass with and without Genshi.

Is the following needed when support for Genshi is removed?: trunk/trac/util/html.py@16037:712-713#L698 .

No, I don't think so.

Should we allow the metadata argument of process_request to be None with removal of Genshi support? From TracDev/PortingFromGenshiToJinja#IRequestHandler:

Note that as long as we have to support the legacy Genshi templates, a None value passed as third argument won't be interpreted as an empty dict, but rather as an empty content_type.

The idea was that once we no longer need to play subtle games to differentiate between the two engines, return 'template.html', data should become equivalent to return 'template.html', data, {}.

in reply to: 9 comment:10 by Ryan J Ollos, 7 years ago

Replying to Christian Boos:

Is the following needed when support for Genshi is removed?: trunk/trac/util/html.py@16037:712-713#L698 .

No, I don't think so.

I've removed the line and will post the changes after some more time passes and another rebase is needed.

Should we allow the metadata argument of process_request to be None with removal of Genshi support? From TracDev/PortingFromGenshiToJinja#IRequestHandler:

I misspoke here. I was looking at render_template and wondering if we should allow metadata to be None. We'd just need to add:

metadata = metadata or {}

comment:16 by Christian Boos, 5 years ago

That clean-up looks good to me, and simplifies things a lot. However, as trunk seems to be still 1.3dev, I suppose you plan to branch 1.4-stable first, right?