Skip to content

Allow to have am empty prefix string, so st2chatops functionality can…#52

Merged
nzlosh merged 3 commits intonzlosh:mainfrom
alagendijk-minddistrict:empy-prefix
Feb 13, 2025
Merged

Allow to have am empty prefix string, so st2chatops functionality can…#52
nzlosh merged 3 commits intonzlosh:mainfrom
alagendijk-minddistrict:empy-prefix

Conversation

@alagendijk-minddistrict
Copy link
Contributor

… be emulated

def _configure_prefixes(self, bot_conf):
self.bot_prefix = bot_conf.BOT_PREFIX
self.plugin_prefix = bot_conf.STACKSTORM.get("plugin_prefix", "st2")
# If there is a plugin prefix set we want a space between it and the command, otherwise not
Copy link
Owner

@nzlosh nzlosh Feb 10, 2025

Choose a reason for hiding this comment

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

The default behaviour is to have plugin_prefix set to st2 when not specified in the configuration. This is to avoid conflicts between existing errbot plugins and stackstorm action-aliases. The proposed change will break behaviour for existing err-stackstorm users, which I'd rather avoid.

To preserve backward compatibility and avoid the space being added to the bot_prefix for people who don't want it, plugin_prefix can be tested as non-zero length and the last character isn't a space, in which case, a space can be appended. There's no need to change the plugin_prefix variable name across the code base.

Here's a small example of what I mean

prefixes=['',None,"st2", "st2 ", "toomany  ", "with space"]
for test in prefixes:
    if test and not test.endswith(" "):
        prefix = f"{test} "
    else:
        prefix = test
    print(f"'{test}' == '{prefix}'")

Produces

'' == ''
'None' == 'None'
'st2' == 'st2 '
'st2 ' == 'st2 '
'toomany  ' == 'toomany  '
'with space' == 'with space '

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My first thought was to do something like that and set the prefix in config, I chose not do the change that way to prevent backwards incompatible changes.
The plugin_prefix is used without a space to register commands for session handling when using user auth for stackstorm.
With the default st2 prefix it registeres commands like "st2session_start", "st2session_end" etc.
When changing the prefix in the config to include the space those would become "st2 session_start", "st2 session_end" etc.

That's why I created a new variable including the space (when needed) to use in the few places where the extra space is currently hardcoded.

Copy link
Owner

Choose a reason for hiding this comment

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

You make a very astute and valid observation. 👍

@nzlosh
Copy link
Owner

nzlosh commented Feb 12, 2025

Can you update the changelog with the prefix behaviour change and the PR can be merged.

@alagendijk-minddistrict
Copy link
Contributor Author

Can you update the changelog with the prefix behaviour change and the PR can be merged.

I have updated the CHANGELOG.md

I also added a small fix for the st2help command, in the message it returns when a alias does not exists it hardcoded referred to !st2help no matter what prefix was set, this now also correctly uses the plugin_prefix

@nzlosh
Copy link
Owner

nzlosh commented Feb 13, 2025

Thanks for this work. 👍

@nzlosh nzlosh merged commit 32fa23a into nzlosh:main Feb 13, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants