IRequestFilter post_process_request req changes not in hdf for ClearSilver

[acamac]

Request changes made in post_process_request are not reflected in hdf. So ClearSilver templates can not see any changes made in req.

Suggested change:

trac/web/main.py

@@ -197 +197 @@
                 resp = chosen_handler.process_request(req)
                 if resp:
                     if len(resp) == 2: # Clearsilver
-                        chrome.populate_hdf(req)
                         template, content_type = \
                                   self._post_process_request(req, *resp)
+                        chrome.populate_hdf(req)
                         # Give the session a chance to persist changes
                         if req.session:
                             req.session.save()

Life Story:

I downloaded a ClearSilver plugin and attempted to use the ​NavControlPlugin to modify the metnav and mainnav. However the changes were not showing up for my ClearSilver generated plugin page.

[Remy Blank]

I understand the problem, but I'm not sure about the implications of the proposed change. Browsing the history of main.py, I can see that chrome.populate_hdf(req) has always been placed before the post-process handler call.

Could somebody who is familiar with ClearSilver and IRequestFilter comment on this?

(OT: A "drill-down" functionality in the annotate view would be very useful to track the history of a set of lines: view the annotated file, display the last changeset affecting the lines "inline", view the annotated file as it was right before the changeset, wash, rinse and repeat. Currently, I'm doing this by editing the URL)

[Christian Boos]

Replying to rblank:

(OT: A "drill-down" functionality in the annotate view would be very useful to track the history of a set of lines: view the annotated file, display the last changeset affecting the lines "inline", view the annotated file as it was right before the changeset, wash, rinse and repeat. Currently, I'm doing this by editing the URL)

Click on the nearest ellipsis corresponding to that set of lines.

[Remy Blank]

Replying to cboos:

Click on the nearest ellipsis corresponding to that set of lines.

Not sure what you mean with "ellipsis". I had found out that clicking on a revision in the "Rev" column would open the corresponding changeset "inline". But I hadn't noticed that the headings of the line number columns in the diff had links to the revisions, so this allows drilling down as I wanted. I assume this is what you meant.

Funnily, your comment:3 didn't appear in my RSS reader. It's in the timeline, so it's probably a problem on my side.

[Christian Boos]

Replying to rblank:

Replying to cboos:

Click on the nearest ellipsis corresponding to that set of lines.

Not sure what you mean with "ellipsis". I had found out that clicking on a revision in the "Rev" column would open the corresponding changeset "inline". But I hadn't noticed that the headings of the line number columns in the diff had links to the revisions, so this allows drilling down as I wanted.

The heading containing the rev number will position you to the first chunk, but if you have multiple chunks, they will be separated by an empty row, the heading of such separators contains an ellipsis, and clicking on that ellipsis will position you to the corresponding chunk.

I think we should add a title attribute to that link (and maybe even a help line somewhere) in order to make this more discoverable.

[Remy Blank]

Replying to cboos:

The heading containing the rev number will position you to the first chunk, but if you have multiple chunks, they will be separated by an empty row, the heading of such separators contains an ellipsis, and clicking on that ellipsis will position you to the corresponding chunk.

Aahh… That's indeed very well hidden :-)

Apart from that, any on-topic comments on the patch above?

[Christian Boos]

Replying to rblank:

Replying to cboos:

The heading containing the rev number will position you to the first chunk, but if you have multiple chunks, they will be separated by an empty row, the heading of such separators contains an ellipsis, and clicking on that ellipsis will position you to the corresponding chunk.

Aahh… That's indeed very well hidden :-)

Indeed, power-user only ;-)

Apart from that, any on-topic comments on the patch above?

Well, from my side, ClearSilver is largely unsupported. Doing changes at that level is risky, as we're not using it anymore at large, and when changing something like proposed here, you may risk to get unwanted side effects.

Especially when, as you said in comment:1, it has always been like that. One can easily imagine that other request post processors would expect to have the HDF populated at the point they're called. The OP could eventually use the request pre processing step to achieve what he wanted. Anywyay, for me, it's not the time to change anymore the behavior of Trac w.r.t ClearSilver, so I'd suggest wontfix.

[Christian Boos]

Replying to cboos:

… One can easily imagine that other request post processors would expect to have the HDF populated at the point they're called. The OP could …

Or simpler, the OP could actually just modify the HDF directly, the way he wants.