EDCD
The plugin link opener is broken for most use cases
I don't know whether to describe this as an enhancement or a bug.
The system for opening a link to a system/station etc. based on user preference is really great.
opener = plug.invoke(url, 'EDSM', 'system_url', monitor.state['SystemName'])
if opener:
return webbrowser.open(opener)
However it's also completely broken by design because it ignores and overrides the system requested and instead opens the current system. The result is that I can't make any use of it at all and have to code my own version bypassing and duplicating the builtin solution.
def system_url(system_name: str) -> str:
"""
Construct an appropriate EDSM URL for the provided system.
:param system_name: Will be overridden with `this.system_address` if that
is set.
:return: The URL, empty if no data was available to construct it.
"""
if this.system_address:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemID64={this.system_address}')
if system_name:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemName={system_name}')
return ''
This is a good idea! Well spotted.
I can patch System and Station to take an optional parameter to accept a custom system or station. Loadout is a bit harder and I don't think there's quite the use case for that so I am not touching that one. (Let me know if there's demand for it though, and I'll do it!)
They already take a system/station parameter, they just override/ignore it!
instead of:
def system_url(system_name: str) -> str:
"""
Construct an appropriate EDSM URL for the provided system.
:param system_name: Will be overridden with `this.system_address` if that
is set.
:return: The URL, empty if no data was available to construct it.
"""
if this.system_address:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemID64={this.system_address}')
if system_name:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemName={system_name}')
return ''
The plugins should read:
def system_url(system_name: str) -> str:
"""
Construct an appropriate EDSM URL for the provided system.
:param system_name: Will NOT be overridden with `this.system_address` if that
is set.
:return: The URL, empty if no data was available to construct it.
"""
if system_name:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemName={system_name}')
if this.system_address:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemID64={this.system_address}')
return ''
The only use case this could break is if some code is passing in a parameter which contains an incorrect value and the code relies on that parameter being ignored/overridden.
OK, I think I see what you're getting at. Take a look at the updated #2446 and let me know if that's more in line with what you're looking for.
The issue isn't opening the URL its RETRIEVING the URL.
Each plugin (EDSM, Inara, Spansh_core) has two functions:
def system_url(system_name: str) -> str:
def station_url(system_name: str, station_name: str) -> str:
You pass these a system name and they return a URL to view that system in your configured tool (EDSM, Inara, or Spansh) and if you don't pass in a system they return the URL for the current system. Excellent, very good.
However that isn't what they actually do. What they actually do is, if there is a current system they completely ignore the system name you passed to them and return a URL for the current system. In other words, if you're logged in to the game they will return one and only one URL (that for the current system) regardless of the parameters passed to them.
So I have code:
# star is a string containing a starsytem that the user wants to view in their preferred system viewer
opener = plug.invoke(config.get_str('system_provider'), 'EDSM', 'system_url', star)
if opener:
return webbrowser.open(opener)
but it is commented out. Because every time I call it it returns the same link -- a link to the current system and not a link to the star that was requested. Your PR will not change that. open_system() will just open the current system regardless of what is passed.
The fix is to change the plugins from (using EDSM.py as an example):
def system_url(system_name: str) -> str:
"""
Construct an appropriate EDSM URL for the provided system.
:param system_name: Will be overridden with `this.system_address` if that
is set.
:return: The URL, empty if no data was available to construct it.
"""
if this.system_address:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemID64={this.system_address}')
if system_name:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemName={system_name}')
return ''
to the following
def system_url(system_name: str) -> str:
"""
Construct an appropriate EDSM URL for the provided system.
:param system_name: Will **NOT** be overridden with `this.system_address` if that
is set.
:return: The URL, empty if no data was available to construct it.
"""
if system_name:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemName={system_name}')
if this.system_address:
return requests.utils.requote_uri(f'https://www.edsm.net/en/system?systemID64={this.system_address}')
return ''
and repeat for station_url() in all the plugins that offer this functionality.
Does that make sense?