Add get_plugins_dir method to Environment class

[Ryan J Ollos]

The proposed change is a refactoring to replace the function trac.loader.get_plugins_dir with the method trac.env.Environment.get_plugins_dir.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t12410.

I stuck with the existing naming convention even though I think these get_* methods that return paths should instead be lazily-evaluated properties.

Should realpath and normpath be called in the similar methods get_templates_dir, get_htdocs_dir, get_log_dir and get_plugins_dir? We could add a _get_path_to_dir method:

def _get_path_to_dir(dir)
    return os.path.normcase(os.path.realpath(os.path.join(self.path, dir)))

def get_plugins_dir(self):
    return self._get_path_to_dir('plugins')

[Ryan J Ollos]

Added comment:1 changes in [962e14e1/rjollos.git].

[Christian Boos]

Sure.

I wonder how many times these methods are called, but now that you added a helper method, it might be a cheap optimization to memoize these calls, e.g.

    def _get_path_to_dir(self, dir):
       """Retrieve and memoize normalized path to immediate subdir of self.path"""
        _dir = '_%s_dir' % dir
        if not hasattr(self, _dir):
            path = os.path.join(self.path, dir)
            setattr(self, _dir, os.path.normcase(os.path.realpath(path)))
        return getattr(self, _dir)

[Ryan J Ollos]

Committed to 1.0-stable in [14613], merged to trunk in [14614]. Deprecated function get_plugins_dir will be removed in #11901.

[Ryan J Ollos]

Replying to Christian Boos:

I wonder how many times these methods are called, but now that you added a helper method, it might be a cheap optimization to memoize these calls, e.g.

In general we seem to follow the undocumented pattern of using properties for short computations or those that can be cached, and get_ for lengthier computations, database access or methods that could raise an exception.

Therefore, for API consistency it might make sense to just deprecate the 4 methods and add lazily-evaluated properties templates_dir, htdocs_dir and plugins_dir, rather than implementing lazy evaluation for the methods. It's unlikely many plugins access those methods anyway, so removing them in 1.3.1 shouldn't have a big impact.

[Ryan J Ollos]

Correction: Technically we only have to deprecate 3 methods since the 4th was just added.

[Ryan J Ollos]

I prepared the changes proposed in comment:7: log:rjollos.git:t12410.2.

[Ryan J Ollos]

Committed additional changes to 1.0-stable in [14637], merged to trunk in [14638].

[Ryan J Ollos]

Additional changes in [14644], [14645].