Ticket #8961 (closed enhancement: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] improve option / default handling, introduce two more options

Reported by: bof Assigned to: ChrisNelson
Priority: normal Component: TracJsGanttPlugin
Severity: normal Keywords:
Cc: rjollos Trac Release: 0.11

Description

This is an enhancement patch I just cooked up. It provides for the following:

  • the builtin defaults for all macro arguments (except query arguments), can now be changed in trac.ini, like this:
    [trac-jsgantt]
    option.openLevel = 0
    
  • all these options, when representing integer values, are type converted, whether coming from trac.ini or macro instantiations. A suitable python conversion error is shown, instead of just not displaying the JS at all (e.g. res=xxx now shows an error.)
  • a new option (trac.ini or macro argument) openClosedTickets, defaulting to 1, can be set to 0 to make parent tickets with status closed, initially show as collapsed, regardless of openLevel setting.
  • a new option (trac.ini or macro argument) userMap, defaulting to 1, can be set to 0 to avoid the conversion of login (ticket owner) names to full / real names in the Resource display column.
    Setting userMap=0 makes Resource show the login names.
    In particular, not doing this conversion, avoids the internal call to self.env.get_known_users(), which could be pretty expensive when you have a huge list of users.

Attachments

tracjsganttplugin-option-defaults.patch (6.7 kB) - added by bof on 07/08/11 18:22:07.
option defaults in trac.ini, new options openClosedTickets, userMap, and omitMilestones

Change History

(in reply to: ↑ description ; follow-up: ↓ 3 ) 07/08/11 15:53:46 changed by ChrisNelson

  • status changed from new to assigned.

Replying to bof:

This is an enhancement patch I just cooked up. It provides for the following: * the builtin defaults for all macro arguments (except query arguments), can now be changed in trac.ini, like this: {{{ [trac-jsgantt] option.openLevel = 0 }}}

Thanks! I've been wanting to do that for a while.

* all these options, when representing integer values, are type converted, whether coming from trac.ini or macro instantiations. A suitable python conversion error is shown, instead of just not displaying the JS at all (e.g. res=xxx now shows an error.)

Sounds like a nice cleanup.

* a new option (trac.ini or macro argument) openClosedTickets, defaulting to 1, can be set to 0 to make parent tickets with status closed, initially show as collapsed, regardless of openLevel setting.

I was planning on adding something like open=open (don't close the tree for open tickets) and have some other thoughts about more flexible open= and close= options. I may not may not adopt or adapt this change.

* a new option (trac.ini or macro argument) userMap, defaulting to 1, can be set to 0 to avoid the conversion of login (ticket owner) names to full / real names in the Resource display column.
Setting userMap=0 makes Resource show the login names.
In particular, not doing this conversion, avoids the internal call to self.env.get_known_users(), which could be pretty expensive when you have a huge list of users.

I can see where that would be useful. Thanks.

07/08/11 16:27:20 changed by bof

Small respin of the patch, adding yet another option:

omitMilestones can be set to 1 to suppress the automatic addition of milestones found on the displayed tickets. Milestones explicitly requested using a milestone= macro argument, will still be presented.

Default for the option is, of course, 0.

I'm replacing the original patch file, because this is both a small addition, and depends on the previous one being applied.

(in reply to: ↑ 1 ) 07/08/11 16:33:28 changed by bof

Replying to ChrisNelson:

Replying to bof:

* a new option (trac.ini or macro argument) openClosedTickets, defaulting to 1, can be set to 0 to make parent tickets with status closed, initially show as collapsed, regardless of openLevel setting.

I was planning on adding something like open=open (don't close the tree for open tickets) and have some other thoughts about more flexible open= and close= options. I may not may not adopt or adapt this change.

Fine with me. I think the (optional) feature to make already closed tickets collapse, is important - I'll probably be happy with a superset solution that lets me be more fancy or selective :)

* a new option (trac.ini or macro argument) userMap, defaulting to 1, can be set to 0 to avoid the conversion of login (ticket owner) names to full / real names in the Resource display column.

I can see where that would be useful. Thanks.

I did not really make that change for the performance improvement. Our login names are a lot shorter than some of the full names, so more horizontal space remains for the ticket summary and status display, when I use the login names.

07/08/11 18:22:07 changed by bof

  • attachment tracjsganttplugin-option-defaults.patch added.

option defaults in trac.ini, new options openClosedTickets, userMap, and omitMilestones

07/08/11 18:24:59 changed by bof

Chris, there was a small bug in the patch; I replaced it with another respin. If you already applied the version from two hours ago, here is the additional change:

@@ -213,7 +219,7 @@
             if not key in self.options:
                 query_args[key] = options[key]

-        if 'root' in options:
+        if options['root']:
             parents = options['root'].split('|')
             query_args['id'] = '|'.join(_children(parents))

Without this change, not using root= results in a python error, because after my previous change to _process_options, now all keys from self.options are present in options, too.

07/10/11 18:23:26 changed by ChrisNelson

(In [10409]) Allow option defaults to be set in trac.ini. Refs #8961.

With this change, option.<opt> in trac.ini overrides the built-in default for <opt>.

Part of a patch from bof. Thanks!

07/10/11 18:23:31 changed by ChrisNelson

(In [10410]) Make user name mapping optional. Refs #8961.

Setting userMap=0 disables mapping of user IDs to full names. (Defaults on.)

Thanks to bof for the patch.

07/10/11 18:23:37 changed by ChrisNelson

(In [10411]) Add omitMilestones option. Refs #8961.

When set to 1, only milestones listed in milestone= will be shown. When set to 0, milestones for all tickets will be shown.

Thanks to bof for the patch.

(follow-up: ↓ 11 ) 08/05/11 19:14:02 changed by ChrisNelson

(In [10559]) Add openClosedTickets option. Refs #8961.

08/05/11 19:14:56 changed by ChrisNelson

  • status changed from assigned to closed.
  • resolution set to fixed.

I haven't gotten to the fancy open= and closed= options I planned and this is a big step forward, covering most of what those would.

08/05/11 20:09:30 changed by rjollos

  • cc set to rjollos.

(in reply to: ↑ 8 ; follow-up: ↓ 12 ) 08/10/11 07:56:38 changed by rjollos

Replying to ChrisNelson:

(In [10559]) Add openClosedTickets option. Refs #8961.

Minor suggestion: I think that the purpose of this option would be way more clear if it was named collapseClosedTickets (and defaulting to 0, of course).

(in reply to: ↑ 11 ; follow-up: ↓ 13 ) 08/10/11 13:51:28 changed by ChrisNelson

  • status changed from closed to reopened.
  • resolution deleted.

Replying to rjollos:

Replying to ChrisNelson:

(In [10559]) Add openClosedTickets option. Refs #8961.

Minor suggestion: I think that the purpose of this option would be way more clear if it was named collapseClosedTickets (and defaulting to 0, of course).

One of my colleagues suggested expandClosedTickets and keep the same sense. Either is clearer than open I think (sorry, bof). Which do you prefer?

(in reply to: ↑ 12 ; follow-up: ↓ 14 ) 08/10/11 14:50:59 changed by bof

Replying to ChrisNelson:

Replying to rjollos:

Replying to ChrisNelson:

(In [10559]) Add openClosedTickets option. Refs #8961.

Minor suggestion: I think that the purpose of this option would be way more clear if it was named collapseClosedTickets (and defaulting to 0, of course).

One of my colleagues suggested expandClosedTickets and keep the same sense. Either is clearer than open I think (sorry, bof). Which do you prefer?

No need to be sorry - I wasn't entirely happy with openClosed, too.

I prefer expandClosedTickets with default 1.

(in reply to: ↑ 13 ) 08/10/11 19:25:43 changed by rjollos

Replying to bof:

I prefer expandClosedTickets with default 1.

+1 for expandClosedTickets.

08/10/11 21:26:44 changed by ChrisNelson

(In [10581]) Rename "openClosedTickets" to "expandClosedTickets". Refs #8961.

08/10/11 21:29:44 changed by ChrisNelson

(In [10583]) Remove unimplented option which snuck into push. Refs #8961.

08/10/11 21:30:25 changed by ChrisNelson

  • status changed from reopened to closed.
  • resolution set to fixed.

Add/Change #8961 ([PATCH] improve option / default handling, introduce two more options)




Change Properties
Action