Also, should I ticket this on t.e.o? (Or in general for patches to core) On Jul 15, 5:05 pm, Jeff Hammel <[EMAIL PROTECTED]> wrote: > 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 > > > tohttp://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... > > > > > >> 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 -~----------~----~----~----~------~----~------~--~---
