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

[Patrick Schaaf]

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.

[Chris Nelson]

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.

[Patrick Schaaf]

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.

[Patrick Schaaf]

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.

[Patrick Schaaf]

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

[Patrick Schaaf]

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.

[Chris Nelson]

(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!

[Chris Nelson]

(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.

[Chris Nelson]

(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.

[Chris Nelson]

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

[Chris Nelson]

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.

[Ryan J Ollos]

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).

[Chris Nelson]

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?

[Patrick Schaaf]

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.

[Ryan J Ollos]

Replying to bof:

I prefer expandClosedTickets with default 1.

+1 for expandClosedTickets.

[Chris Nelson]

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

[Chris Nelson]

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