Opened 23 months ago
Last modified 11 months ago
#9064 assigned enhancement
More rendering options for ListTagged macro and tag-handler
| Reported by: | anonymous | Owned by: | hasienda |
|---|---|---|---|
| Priority: | normal | Component: | TagsPlugin |
| Severity: | normal | Keywords: | columns format selection |
| Cc: | itamarost, rjollos | Trac Release: | 0.11 |
Description
Two extra rendering features I'd like to have:
- [[ListTagged(query,format=short)]]: Don't list tags for every item, use the description OR resource-id as caption (not both)
- [[ListTagged(query,format=table,cols=realm|id|description|tags)]]: Show the results in a table with the specified columns in the specified order.
Also support using these formats when rendering the tag-handler with a query.
Use config-options to determine default formats and table-columns (independently for the macro and the handler), and allow overriding with explicit arguments to the macro.
The patch in #9059 implements these features.
Attachments (2)
Change History (29)
comment:1 Changed 22 months ago by hasienda
- Cc itamarost added
- Keywords columns format selection added
- Status changed from new to assigned
comment:2 Changed 22 months ago by hasienda
Oh, sorry, didn't look close enough to spot you had different options in mind - my fault.
comment:3 Changed 21 months ago by hasienda
Another observation: Using the web-UI (search form in main tags page '/tags', if you provide tags to search for, it switches from calling Cloud to ListTagged behind the scenes (of course quite obvious by the result presentation as list vs. cloud).
BUT with your patch in this situation the settings for ListTagged would overrule, instead of persisting the generic settings and using letting ListTagged settings rule direct wiki macro invocations. What do you think? Am I mistaken somewhere somehow?
comment:4 Changed 21 months ago by hasienda
I'm almost done with this, executing just more tests now, but would still value your feedback on the last question.
As a side-note: The TagsPlugin has got i18n support since 0.7dev, so I dropped suggested hard-coded table column names in favor of translatable column labels. Your alternative ListTagged presentations rock, and localized column labels are the suitable topping.
This last part has been the most enjoyable and exiting development part of all, maybe close to the Tags admin feature.
comment:5 Changed 21 months ago by hasienda
Any word on my settings-override question?
ListTagged rocks with your new table and a minor tweak to improve ticket description formatting, that I'll add on-top. Would like to commit this soon.
comment:6 follow-up: ↓ 7 Changed 21 months ago by itamarost
Sorry for the delayed response.
If I understand correctly, you raise a concern about the listtagged default settings overriding the default generic settings when filtering tags in the /tags handler.
In the patch I tried addressing this concern. In web_ui.py, in TagRequestHandler::process_request(), before invoking macros.expand_macro(), I inject the default generic settings as explicit parameters in the query if the macro about to be invoked is ListTagged.
Of course it is possible that I overlooked something, and this is not sufficient in cases I missed. I'd love to know about such missed cases.
Besides that - it's great that you added i18n support! I'm really glad you like the enhancements and put so much effort into polishing the patch and merging it into trunk! Thanks!
comment:7 in reply to: ↑ 6 Changed 21 months ago by hasienda
Replying to itamarost:
Sorry for the delayed response.
No problem so far. Code is normally only getting better while lying in my local patch stack (Mercurial Queues). Already catched one flaw or another meanwhile.
If I understand correctly, you raise a concern about the listtagged default settings overriding the default generic settings when filtering tags in the /tags handler.
Almost. More about the realm selection.
In the patch I tried addressing this concern. In web_ui.py, in TagRequestHandler::process_request(), before invoking macros.expand_macro(), I inject the default generic settings as explicit parameters in the query if the macro about to be invoked is ListTagged.
Yes I see, and this exposed your care-taking for possible argument overwrites indeed.
Of course it is possible that I overlooked something, and this is not sufficient in cases I missed. I'd love to know about such missed cases.
Well, I found that some realm preselection didn't make it into the re-used ListTagged invocation. If done via TagsQuery arguments, yes, but not when using the checkboxes near the input field, and this is certainly the dominant way of realm selection for the generic TagsQuery. In marco.py the ListTagged setting was always considered by checking against self.exclude_realms. I think I've got that fixed by introducing realms as the third kw argument.
Besides that - it's great that you added i18n support! I'm really glad you like the enhancements and put so much effort into polishing the patch and merging it into trunk! Thanks!
You're welcome. Great - so we get the most out of it. I think I'm way better in modifying and extending existing stuff than in creating something totally new (with great, forward-looking structure) out of thin air. Maybe we can further cooperate, i.e. with the issue I'll raise at th-users in the next few minutes about Tags quoting? Would really love to hear from you about that as well.
comment:8 Changed 21 months ago by hasienda
(In [10680]) TagsPlugin: Provide default sets of two new options for both, ListTagged and /tags handler, refs #9059 and #9064.
The TagQuery result presentation is now configurable. Supported values for
the format keyword argument are:
- oldlist - list in known ListTagged style
- compact - list with results shortend to just the description
- table - table view similar to TicketQuery with 'table' option
and for 'table' view columns can be selected with the cols argument from a
frozenset(['realm', 'id', 'description', 'tags'])
separating multiple values by pipe character ('|').
While these are great new features I wanted myself before, it would not have
happend by now without the contribution from Itamar Ostricher. Thanks a bunch.
comment:9 Changed 21 months ago by hasienda
Itamar, you may notice, that I changed one of your suggested values for format argument.
Initially I thought of using known values, so replaced 'short' with 'compact' as used for format argument in modules of Trac's ResourceSystem class (see get_resource_description). Later I recognized, that for now it only matters to the check embedded to the ListOption itself.
Anyway, I just meant to warn you in case you'll replace your own patched plugin with trunk and already used your value before.
comment:10 follow-up: ↓ 11 Changed 21 months ago by itamarost
Nice work!
Thanks for the warning about the short->compact. It affects only my plugin-dev environment, so I'll survive :-) .
You mentioned Tags quoting in th-users. Haven't seen such a post. Did you send?
comment:11 in reply to: ↑ 10 Changed 21 months ago by hasienda
comment:12 Changed 21 months ago by hasienda
(In [10737]) TagsPlugin: Wrap ListTagged results in Trac pager, refs #4503 and #9064.
This fixes SystemError: ../Objects/tupleobject.c ... occasionally seen
with large sets of results. And response time is getting reasonable even
for 30.000+ matching tagged resources.
Beware: Hold on, it's a feature-incomplete changeset - a step in transition.
comment:13 Changed 21 months ago by hasienda
(In [10738]) TagsPlugin: Move Genshi builder instructions to fragment template, refs #4503 and #9064.
Now all list formats and even the warning message about obsolete TagsQuery
arguments are integrated into a HTML fragment template. Various goodies like
differently shaded odd/even rows and 'no results message' have been added.
Oddly squeezed ListTagged tables are gone as well - all due to re-using big
parts of the TicketQuery code from Trac core.
comment:14 Changed 21 months ago by rjollos
- Cc itamarost. rjollos added; itamarost removed
comment:15 Changed 21 months ago by hasienda
- Cc itamarost added; itamarost. removed
Certainly you didn't want to add that dot. :-) Dunno what this could cause to the validity of Itamar's reference.
comment:16 Changed 19 months ago by AdrianFritz
Tested and worked good under Trac 0.12 + PG (as database) + TagsPlugin (TracTags 0.7dev-r10764) many more plugins. Documented under "dev status/version" at wiki:TagsPlugin?version=54 (version column must be updated when release version 0.7).
comment:17 follow-up: ↓ 19 Changed 19 months ago by AdrianFritz
P/S: is there a way to allow copying a selected tag from "Current Tag" (under /admin/tags/replace tab). Use Case: avoid new mistype when required to rename a tag, then you copy from and edits before changing.
comment:18 follow-up: ↓ 21 Changed 19 months ago by hasienda
Thank you for all the wiki page re-factoring. I think, you made some good decisions, but let's consider the <realm> vs. <resource> topic here, please:
The meaning in source is realm='ticket' and resource='ticket:9064. While realm establishes a name-space, a resource is just one document in it. Trac core has exactly these identifies for it's resource system. May we agree and adjust the user-facing docs like our wiki page accordingly?
comment:19 in reply to: ↑ 17 ; follow-up: ↓ 20 Changed 19 months ago by hasienda
Replying to AdrianFritz:
P/S: is there a way to allow copying a selected tag from "Current Tag" (under /admin/tags/replace tab). Use Case: avoid new mistype when required to rename a tag, then you copy from and edits before changing.
You want to pre-set the 'New Tag:' input field with the selected tag, if only one is selected, right? Hmm, this won't work without some dynamic HTML (read: JavaScript or AJAX). But sure, let me think about it. Wonder how complex your tags must be, if you ask for auto-copying them. Care to provide a patch?
But at any rate, I sincerely ask you to restart this enhancement request within a new ticket. Changes for this one have already grown enough in complexity. And be aware, that I see much more pressing issues, like insane quoting (see #3624 and #9057),
comment:20 in reply to: ↑ 19 Changed 19 months ago by AdrianFritz
Replying to hasienda:
Replying to AdrianFritz:
P/S: is there a way to allow copying a selected tag from "Current Tag" ......
.... to restart this enhancement request within a new ticket.
Sure, done @ #9574
comment:21 in reply to: ↑ 18 Changed 19 months ago by anonymous
Replying to hasienda:
... let's consider the <realm> vs. <resource> topic here, please:
The meaning in source is realm='ticket' and resource='ticket:9064. While realm establishes a name-space, a resource is just one document in it. Trac core has exactly these identifies for it's resource system. May we agree and adjust the user-facing docs like our wiki page accordingly?
Thank-you very much for your guidance. Under this explanation, how about adopting "<name-space>" instead of "<realm>". i.e. where actually read "realm:<resource>" will be "realm:<name-space>". Please, correct me if different than this.
comment:22 follow-ups: ↓ 23 ↓ 24 Changed 19 months ago by hasienda
You'll need to explain 'name-space' too, so why not stick to 'realm'?
I'm fine with 'name-space', but would still suggest to stick to 'realm' and explain it in this context for reasons like the one mentioned above. For Trac developers 'realm' is perfectly clear, and they are especially responsible for keeping the code in good state and docs current as well. Another word ('name-space') alone wouldn't make a difference, but could potentially confuse someone who already knows about Trac realms beyond what we have now without a (prominent) explanation.
comment:23 in reply to: ↑ 22 Changed 19 months ago by rjollos
Replying to hasienda:
You'll need to explain 'name-space' too, so why not stick to 'realm'?
+1 for keeping with standard Trac terminology such as realm. More documentation describing realm and resource should be helpful.
comment:24 in reply to: ↑ 22 ; follow-up: ↓ 25 Changed 19 months ago by AdrianFritz
Replying to hasienda:
You'll need to explain 'name-space' too, so why not stick to 'realm'?
Thanks again for your explanation. Now I understand a bit more. Sure, let´s keep aligned with real. Reviewed wiki:TagsPlugin?version=56 with also more details for cols option.
comment:25 in reply to: ↑ 24 Changed 19 months ago by AdrianFritz
Sorry, "...let´s keep aligned with realm."
comment:26 Changed 18 months ago by itamarost
I attached a patch that allows format selection in the /tags handler using URL options.
comment:27 Changed 18 months ago by itamarost
I have attached another patch, to address an issue that seems like a regression to me, following r10738, where the use of listtagged_results template was introduced.
The regression: When using compact format, ListTagged used the resource id's instead of description (when available).
In addition, when using the compact format with tickets, I think the resource description as shown in the generated list should include a "Ticket #num:" prefix for clarity, so I also added that.
A note on headings duplicity:
If two different wiki pages have the same heading and share a tag, then a ListTagged on the shared tag will result items in the list with identical text (but different links). I think it is OK, as identical heading usually means someone copied a page without updating the heading.
A note on headings with wiki syntax:
It is possible to use wiki syntax in page heading (e.g. "= A [wiki:Page Page] with some WikiSyntax #4" contains 3 links).
The current implementation ignores this, and takes the heading as raw text.
I tried addressing this by using the format_to_oneliner function, but that made things worse as the links in the heading resolved OK but then the entire description couldn't be linked to the tagged page itself... (this can be seen in the commented lines in the patch)
So I decided to leave it as it is now, assuming linking from heading is not common.
Thoughts on above notes are welcome.


Collecting the relevant parts from your cumulative patch now.
Don't define any Trac Option more than once. There has to be just one single place, or the behavior of Trac's option auto-discovery on plugin load is undetermined - hard to see from testing, I know. But this is one lesson I had to learn on my own, a while back with WikiTicketCalendarMacro.