Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace {version} in launch args #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Replace {version} in launch args #25

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3580ff
Merge remote-tracking branch 'refs/remotes/shotgunsoftware/master'
Nov 10, 2015
1c67223
Replace {version} token in app args
Nov 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,23 @@ def _get_app_path(self, version=None):
else:
return self._translate_version_tokens(raw_app_path, version)

def _get_app_args(self, version=None):
""" Return the platform specific app args, performing version substitution. """
platform_name = {"linux2": "linux", "darwin": "mac", "win32": "windows"}[sys.platform]
raw_app_args = self.get_setting("%s_args" % platform_name, "")
if version is None:
# there are two reasons version could be None
# the first is if versions have not been configured, in which case the raw args are valid
# if versions has been configured, then we should expand with the first element in the
# list, which will be treated as the default
versions = self.get_setting("versions")
if versions:
return self._translate_version_tokens(raw_app_args, versions[0])
else:
return raw_app_args
else:
return self._translate_version_tokens(raw_app_args, version)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than duplicating the code that does the version replacement logic, how about breaking that out to it's own method as well? Something like this (untested):

    def _apply_version_to_setting(self, raw_string, version=None):
        """
        Replace any version tokens  contained in the raw_string with the appropriate version value from 
        the app settings. 

        If version is None, then no version has been specified and we will return the first version 
        from the versions list in the settings. If no versions have been defined in the settings, then 
        we return the raw_string since there's no replacement to do.

        :param raw_string: the raw string potentially containing the version tokens (eg. {version}, 
                           {v0}, ...) we will be replacing. This string could represent a number of 
                           things including a path, an args string, etc. 
        :param version: version string to use for the token replacement. 
        :returns: string with version tokens replaced with their appropriate values
        """
        if version is None:
            # there are two reasons version could be None
            # 1. if versions have not been configured, the raw string is assumed valid
            # 2. if versions has been configured, but no specific version was requested, then we 
            #    expand with the first element in the versions list, and use it as the default
            versions = self.get_setting("versions")
            if versions:
               return self._translate_version_tokens(raw_string, versions[0])
            else:
               return raw_string
        else:
            return self._translate_version_tokens(raw_string, version)

    def _get_app_path(self, version=None):
        """ Return the platform specific app path, performing version substitution. """
        platform_name = {"linux2": "linux", "darwin": "mac", "win32": "windows"}[sys.platform]
        raw_app_path = self.get_setting("%s_path" % platform_name, "")

        return self._apply_version_to_setting(raw_app_path)

    def _get_app_args(self, version=None):
        """ Return the platform specific app path, performing version substitution. """
        platform_name = {"linux2": "linux", "darwin": "mac", "win32": "windows"}[sys.platform]
        raw_app_args = self.get_setting("%s_args" % platform_name, "")

        return self._apply_version_to_setting(raw_app_args)

def _launch_app(self, context, file_to_open=None, version=None):
"""
Launches an application. No environment variable change is leaked to the outside world.
Expand Down Expand Up @@ -279,7 +296,7 @@ def _launch_app_internal(self, context, file_to_open=None, version=None):

# get the app args:
platform_name = {"linux2": "linux", "darwin": "mac", "win32": "windows"}[sys.platform]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, I don't think we need the platform_name anymore in this scope.

app_args = self.get_setting("%s_args" % platform_name, "")
app_args = self._get_app_args(version)

engine_name = self.get_setting("engine")
if engine_name:
Expand Down