On Mon, Jul 14, 2008 at 07:12:48PM +0200, Christopher Lenz wrote: > > On 02.07.2008, at 17:49, Jeff Hammel wrote: > > On Tue, Jul 01, 2008 at 10:35:01PM +0200, Christopher Lenz wrote: > >> On the patch itself: > >> > >> * What's the use case for configurable default query_args? Couldn't > >> the default options for the query module itself be reused instead, if > >> possible? Or just some sane default like all open tickets? > > > > Now I'm using [query]\ndefault_anonymous_query. Is that better? I > > don't want to use default_query as one of these fields is (by > > default) owner. > > Good point. default_anonymous_query should work for now, we can also > switch to a separate config option, or even a sensible hard-coded > default, later if needed.
I'm neutral on whether this should be its own config option. Some trac admins might want this; to others, its just yet another config option that needs tweaking. So I can go either way. > >> * The excluded_fields option refers to fields not included with Trac > >> core. This should probably default to an empty list instead, if it is > >> even necessary at all (note how I like to avoid adding config > >> options :P ) > > > > I've left this in for now. This is a solution to > > http://trac-hacks.org/ticket/3178 > > which is necessary since the timingandestimation plugin uses JS to > > markup the fields. A couple of alternatives here: > > > > * take out the excluded_fields entirely; this will make the patch > > cleaner, but will break timingandestimation. If the patch is > > committed to core, then I'll ticket that plugin noting the issue and > > maybe try to make a patch. > > > > * instead of having an option, just have these hard-coded fields > > with a "# XXX" noting the reason that this is here. Again, its > > probably worth alerting timingandestimation about this so we can > > kill this entirely. > > Yeah, this really should be fixed in TimingAndEstimation. Providing an > excluded_fields option definitely makes sense, but the name needs > improvement as it sounds way too broad… maybe something like > "unlinked_fields" or such? Done, its now unlinked_fields. +1 on fixing it in TimingAndEstimation. > >> * The query_link thing is kind of brute force. Would be nicer to just > >> use the **kwargs of req.href to construct the URL qith query string > >> params. And it probably should be `query_link(owner=ticket.owner)` > >> instead of `query_link('owner', ticket.owner)`. It would probably be > >> possible to have query_link be just a partial binding of req.href. > > > > I've kept this as a function in web_ui.py, though it does feel a bit > > brute force. I haven't changed it to be (**kw) since it will always > > take one key, value pair. If its really better to have it use **kw, > > I can do this. > > Makes sense. > > > Also, maybe its better just to put this function in the template? > > Another alternative is just to markup the fields themselves and not > > bother with the function at all. Any thoughts on this? > > On second though, the query_link stuff could probably be moved into > TicketModule._prepare_fields() and the template would just access > field members. That'd also move the excluded_fields logic out of the > template and into the controller. That feels slightly cleaner to me, > but no biggie either way. I've done this and it definately feels cleaner. All the logic now lives in web_ui.py and there are only two lines changed in the template (due to the owner and reporter fields being "special"). I'll paste the updated patch at EOM, or it is available here: http://trac-hacks.org/attachment/ticket/3226/autoquery_patch_vs_r7349.txt > >> On a related note, I'd prefer if the links in the ticket properties > >> sbox (the yellow box) would be more subtle (I'm assuming they're the > >> default red right now), possibly even only really discoverable on > >> hover. Many pages in Trac have already gotten way to "linky"; at some > >> point you end up with content where every second word is a red link, > >> which is ugly and distracting :P > > > > They are indeed the default red. > > Just to play Devil's advocate, I don't mind super-linky pages and > > actually like them. But I'm not against making these links more > > subtle. Is there an existing class that I can use to make these > > more subtle? I don't mind adding a class, but I don't really have > > much aptitude for UI, so any CSS I would do probably wouldn't be > > wonderful. Still I can try if you like. > > Never mind, you can just leave this as is. I (or someone else) might > get a chance to make "non-essential" links more subtle sometime in the > future. > > Thanks, > -- > Christopher Lenz > cmlenz at gmx.de > http://www.cmlenz.net/ Thank you. Keep me updated on the status here -- I'd love to see this in core. Jeff The patch: Index: trac/ticket/web_ui.py =================================================================== --- trac/ticket/web_ui.py (revision 7349) +++ trac/ticket/web_ui.py (working copy) @@ -26,7 +26,7 @@ from genshi.builder import tag from trac.attachment import AttachmentModule -from trac.config import BoolOption, Option, IntOption, _TRUE_VALUES +from trac.config import BoolOption, Option, IntOption, ListOption, _TRUE_VALUES from trac.core import * from trac.mimeview.api import Mimeview, IContentConverter, Context from trac.resource import Resource, get_resource_url, \ @@ -105,6 +105,10 @@ If set to 'default', this is equivalent to 'yes' for new environments but keeps the old behavior for upgraded environments (i.e. 'no'). (''since 0.11'').""") + + unlinked_fields = ListOption('ticket', 'unlinked_fields', + default=['estimatedhours', 'hours', 'totalhours'], + doc="fields to exclude from AutoQuery markup") # IContentConverter methods @@ -1029,6 +1033,14 @@ for key in field_changes: ticket[key] = field_changes[key]['new'] + def _query_link(self, req, name, value): + """return a link to /query with the appropriate name and value""" + query = req.href('query', **{name:value}) + args = self.env.config.get('query', 'default_anonymous_query') + if args: + query = '%s&%s' % (query, args) + return tag.a(value, href=query) + def _prepare_fields(self, req, ticket): context = Context.from_request(req, ticket.resource) fields = [] @@ -1036,6 +1048,10 @@ name = field['name'] type_ = field['type'] + # enable a link to custom query for the field + if name not in self.unlinked_fields: + field['rendered'] = self._query_link(req, name, ticket[name]) + # per field settings if name in ('summary', 'reporter', 'description', 'status', 'resolution'): @@ -1226,7 +1242,9 @@ 'attachments': AttachmentModule(self.env).attachment_data(context), 'action_controls': action_controls, 'action': selected_action, - 'change_preview': change_preview + 'change_preview': change_preview, + 'reporter_link': self._query_link(req, 'reporter', ticket['reporter']), + 'owner_link': self._query_link(req, 'owner', ticket['owner']) }) def rendered_changelog_entries(self, req, ticket, when=None): Index: trac/ticket/templates/ticket.html =================================================================== --- trac/ticket/templates/ticket.html (revision 7349) +++ trac/ticket/templates/ticket.html (working copy) @@ -135,9 +135,9 @@ not in ('type', 'owner')]"> <tr> <th id="h_reporter">Reported by:</th> - <td headers="h_reporter" class="searchable">${authorinfo(ticket.reporter)}</td> + <td headers="h_reporter" class="searchable">${reporter_link}</td> <th id="h_owner">Owned by:</th> - <td headers="h_owner">${ticket.owner and authorinfo(ticket.owner) or ''} + <td headers="h_owner">${owner_link} </td> </tr> <tr py:for="row in group(fields, 2, lambda f: f.type != 'textarea')" --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Trac Development" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/trac-dev?hl=en -~----------~----~----~----~------~----~------~--~---
