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.

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

Add params support for aliases #1900

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

H3rnand3zzz
Copy link
Contributor

See description

  • I ran valgrind when using my new feature

How to test the functionality

  • Add an alias that can be used with params, e.g.
    /alias add m /msg
    use alias like a normal command (/m friend Hello!)

@jubalh jubalh added the feature label Oct 19, 2023
@jubalh jubalh added this to the next milestone Oct 19, 2023
@jubalh
Copy link
Member

jubalh commented Oct 19, 2023

Cool feature!

This seems to remove support to use alias a a quick way to write messages or your tatus and so on.
Like in the (old) example: /alias add a /away /alias add a /away \"I'm in a meeting.\"",.

I think users most likely will use this feature and it shouldn't be dropped. Most importantly if such a feature is dropped it should be mentioned in the commit message :)

I know this arguments parsing is quite annoying stuff. I even believe we have this in more commands (the task/question on how to deal with spaces and ""). Do you think you could rewrite this PR to support parameters but still support the old behaviour?

Some ideas:

  • have a new subcommand /alias strict-add that doesn't allow spaces like /alias add a /away \"I'm in a meeting.\"", but will take it as an argument (strict-add might be a bad name. I didn't take much time to think about it since I'm tired).
  • Somehow solve it with "" or let's say if you find a " in the pattern then it will treat it differently

@jubalh jubalh requested a review from sjaeckel October 19, 2023 21:11
@H3rnand3zzz
Copy link
Contributor Author

H3rnand3zzz commented Oct 19, 2023

Cool feature!

This seems to remove support to use alias a a quick way to write messages or your tatus and so on. Like in the (old) example: /alias add a /away /alias add a /away \"I'm in a meeting.\"",.

I think users most likely will use this feature and it shouldn't be dropped. Most importantly if such a feature is dropped it should be mentioned in the commit message :)

I know this arguments parsing is quite annoying stuff. I even believe we have this in more commands (the task/question on how to deal with spaces and ""). Do you think you could rewrite this PR to support parameters but still support the old behaviour?

Some ideas:

* have a new subcommand `/alias strict-add` that doesn't allow spaces like `/alias add a /away \"I'm in a meeting.\"",`  but will take it as an argument (strict-add might be a bad name. I didn't take much time to think about it since I'm tired).

* Somehow solve it with "" or let's say _if_ you find a `"` in the pattern then it will treat it differently

it does not drop support, just "away" command is gone now. You don't need to use quotes, it works the same as before, also it supports quotes for params

@jubalh
Copy link
Member

jubalh commented Oct 20, 2023

it works the same as before

Ahh great! As mentioned I was not sure about it since I was quite tired last night. I will take a look deeper now.

just "away" command is gone now.

It moved to /status set away. Would be great if you could adapt this.

src/command/cmd_defs.c Outdated Show resolved Hide resolved
Before aliases used spaces in their name,
now the alias part is being read only before the first space,
thus allowing execution of complex command with aliases.

Example (with plugin):
`/alias add echo "/system exec echo"`
will allow execution of
`/echo test`
as opposed to prior state when the Profanity will
search for alias "echo tests" and output `Unknown command: /echo test`

Minor change: removed an example with invalid command (`/away`)
@H3rnand3zzz H3rnand3zzz force-pushed the enhancement/better-aliases branch from 7bd12ea to 9ecade9 Compare October 20, 2023 08:42
@jubalh jubalh merged commit 0664491 into profanity-im:master Oct 20, 2023
6 checks passed
@H3rnand3zzz H3rnand3zzz deleted the enhancement/better-aliases branch November 6, 2023 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants