Modify

Opened 3 years ago

Last modified 5 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:

  1. [[ListTagged(query,format=short)]]: Don't list tags for every item, use the description OR resource-id as caption (not both)
  2. [[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)

tagsplugin-9064-itamaro-v1.patch (882 bytes) - added by itamarost 3 years ago.
patch against tagsplugin trunk
tagsplugin-9064-formatting-itamaro-v1.patch (2.2 KB) - added by itamarost 3 years ago.
patch against tagsplugin trunk

Download all attachments as: .zip

Change History (31)

comment:1 Changed 3 years ago by hasienda

  • Cc itamarost added
  • Keywords columns format selection added
  • Status changed from new to assigned

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.

comment:2 Changed 3 years ago by hasienda

Oh, sorry, didn't look close enough to spot you had different options in mind - my fault.

comment:3 Changed 3 years 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 3 years 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 3 years 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: Changed 3 years 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 3 years 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 3 years 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 3 years 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: Changed 3 years 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 3 years ago by hasienda

Replying to itamarost:

Nice work!

You're welcome. Feels good to hear that indeed.

You mentioned Tags quoting in th-users. Haven't seen such a post. Did you send?

Got hold back, but did it now. :-)

comment:12 Changed 3 years 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 3 years 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 3 years ago by rjollos

  • Cc itamarost. rjollos added; itamarost removed

comment:15 Changed 3 years 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 3 years 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: Changed 3 years 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: Changed 3 years 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: Changed 3 years 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 3 years 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 3 years 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: Changed 3 years 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 3 years 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: Changed 3 years 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 3 years ago by AdrianFritz

Sorry, "...let´s keep aligned with realm."

Changed 3 years ago by itamarost

patch against tagsplugin trunk

comment:26 Changed 3 years ago by itamarost

I attached a patch that allows format selection in the /tags handler using URL options.

Changed 3 years ago by itamarost

patch against tagsplugin trunk

comment:27 Changed 3 years 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.

comment:28 Changed 5 months ago by hasienda

In 13731:

TagsPlugin: Wikify descriptions in tagged resource listings, refs #9064 and #11274.

comment:29 Changed 5 months ago by hasienda

In 13815:

TagsPlugin: Completing preparation for v0.7 release.

Availability of that code as stable, tagged release
closes #2429, #3359, #3610, #3624, #3677, #3754, #3864, #3947, #3983, #4078, #4277, #4503, #4799, #5523, #7787, #7857, #8638, #9057, #9058, #9059, #9060, #9061, #9062, #9063, #9149, #9210, #9521, #9630, #9636, #10032, #10416, #10636, #11096, #11147, #11152, #11274, #11302, #11658 and #11659.

Additionally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily, specifically
refs #2804, #4200, #8747 and #9064.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

Add Comment

Modify Ticket

Action
as assigned .
Author


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

 
Note: See TracTickets for help on using tickets.