Skip to content

Commit

Permalink
Notify about absent replacement placeholders in custom commands
Browse files Browse the repository at this point in the history 
Such as `--open-command='xdg-open "%u"'`

Closes #142
  • Loading branch information
@exquo
exquo committed Oct 21, 2021
1 parent cf7c06f commit 1329e23
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions scli
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ def get_urls(txt):
def callf(cmd, rmap=None, background=False, **subprocess_kwargs):
if rmap:
for key, val in rmap.items():
if key not in cmd:
raise ValueError(f'Command string `{cmd}` should contain a replacement placeholder `{key}` (e.g. `some-cmd "{key}"`). See `--help`.')
cmd = cmd.replace(key, val)

if not subprocess_kwargs.get('shell'):
Expand Down

3 comments on commit 1329e23

@Jehops
Copy link
Contributor

@Jehops Jehops commented on 1329e23 Nov 5, 2021

Choose a reason for hiding this comment

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

I have the following in ~/.config/sclirc

notification-command=flite 'Signal message from %s'

After this change, the notification no longer works and I see the error about not using a placeholder.

@exquo
Copy link
Collaborator Author

@exquo exquo commented on 1329e23 Nov 7, 2021

Choose a reason for hiding this comment

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

Thanks for reporting this!

It happens because the --notification-command expects two replacement placeholders: %s for the sender, and %m for the message text. However, omitting one of them, as in your example, should be perfectly valid for this command.

I'll make some changes to indicate to callf that some of the placeholders in the command string are optional.

@exquo
Copy link
Collaborator Author

@exquo exquo commented on 1329e23 Nov 8, 2021

Choose a reason for hiding this comment

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

Should be fixed in 6cb807d

Please sign in to comment.