240114 - Add variables/parameters to aliases

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, enhancement)

Description

[Carlos Carvalho]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8

The Alias function you build in chatzilla is already pretty neat. Typing just
/wb I can say "Welcome back, my old friend". If I add a word after /wb, it will
say this word again after the message.

What if I want so say "Welcome back Carlos, my old friend"? Would it be possible
to add a variable to the script? So by typing /wb Carlos I could get the above
message.

The script could look like this "wb $1 = msg #channel Welcome back $1, my old
friend".

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[:Gijs (he/him)]

Attachment: Proposed patch to add parameters to the alias function

Variables that may be used:

$(n) --> gets replaced with parameter n. 
$(n+) --> gets replaced with parameters n and onwards. Note that $(1+) equals
$(all) BUT $(1+) *requires* at least 1 parameter to be present, whereas $(all)
will not complain be complained about if no parameters are passed.
$(n-p) --> gets replaced with parameters n through p (this includes both
parameters n and p). n does not necessarily need to be higher than p.
$(nick) --> gets replaced with your current nick on the current server.
Therefore, it needs a connected server.
$(recip) --> gets replaced with the channel OR user that you're currently
talking to (recip is supposed to be short for recipient). Ergo, if you include
this in your alias, it will require a channel or user.
$(all) --> returns all the parameters separated by spaces.

Notes:
* The alias will not be executed if parameters are asked for but not supplied.
  Ex: poke = me pokes $(1) will not be run if there are no parameters supplied.

  Instead, an error will be thrown (This alias requires at least 1 parameters.)

* The alias will warn you if you provide more parameters than necessary, just
like a normal command will.
  Ex: poke = me pokes $(1) will warn if you provide two or more parameters.
* General examples:

poke = me pokes $(1)
slap = me slaps $(1) around a bit with $(2+)
quiet = me thinks $(recip) is a bit quiet right now.
etc. etc.

I hope everything works as it should, it did when I last tested it ;).

Comment 2

[:Gijs (he/him)]

Comment on attachment 167105 [details] [diff] [review]
Proposed patch to add parameters to the alias function

Requesting review.

Comment 3

[:Gijs (he/him)]

Attachment: Update of 167105: Proposed patch to add parameters/variables to the alias function

The previous patch did not throw a warning when supplying parameters to a
command that doesn't request any parameters or variables (no $ sign is present
in the line). Fixed now.

Comment 4

[:Gijs (he/him)]

Attachment: style changes, only 1 replace now, added code to make standard aliases work

Changes:
- The built-in aliases should just work again like they used to do now. This
does mean that if you want to define an alias with more than one command (ie:
you have an unescaped ; in the commandtext) you have to use the variables like
$(1) etc. if you want to use the parameters the user passes. If you have a
single-command alias, it will get the parameters the user passes the same way
that it used to do.
- There is only 1 replace and 1 lambda function left
- There are some changes for style's sake, it should be according to the style
rules now (I hope).

Comment 5

[:Gijs (he/him)]

Attachment: More style changes to the previous patch

More style changes to the bug. Couple of unnecessary if's etc. etc.

Comment 6

[James Ross]

Comment on attachment 169135 [details] [diff] [review]
More style changes to the previous patch

>Index: extensions/irc/xul/content/commands.js
>+    function parseAlias(aliasLine, e) {
>+        const ALIAS_ERR_REQ_PRMS = 1;
>+        const ALIAS_ERR_REQ_SRV = 2;
>+        const ALIAS_ERR_REQ_RECIP = 3;

Might be nice to comment that the higher valued errors hide lower-valued ones
(the use of Math.max later on).


>+        if (!aliasLine.match(/\$/))
>+        {
>+            if (e.inputData != "")

Don't strictly need the != "" part here.


>+            if (start && end)
>+            {
>+                // Ranged replace: parameters start through end
>+                start--; //'decrement to correct 0-based index.
>+                if (start >= end) 
>+                {
>+                    var iTemp = end - 1;
>+                    end = start + 1;
>+                    start = iTemp;
>+                    // We obviously have a very stupid user, but we're nice
>+                }

How about moving the start-- to after the if, so you don't need those -1/+1
bits? (you'd also want to change the if from >= to >)


>+            if (nick)
>+            {
>+                // Replace with own nickname
>+                if (e.network && e.server && e.network.state == NET_ONLINE)
>+                {
>+                    return e.server.me.unicodeName;
>+                }
>+                else

"else after return"


>+                // Replace with current recipient
>+                if (e.channel) 
>+                {
>+                    return e.channel.unicodeName;
>+                } 
>+                else if (e.user) 

Same.

>+                {
>+                    return e.user.unicodeName;
>+                } 
>+                else

And again.


>+             // Replace with all parameters
>+             paramsUsed = parameters.length;
>+             return parameters.join(" ");
>+        }

Robert suggested I put a ";" at the end of nested functions, so probably a good
idea to tell you too as well. :)


>+        // If the replace function has a problem, this is an error constant:
>+        var errorMsg = 0;
>+        var paramsUsed = 0;
>+        var maxParamsAsked = 0;
>+        
>+        // set parameters array and prevent line breaks from messing things up

"line breaks"? Could you be clearer about what you're doing? (I think you're
"escaping unsafe characters" or something)

>+        e.inputData = e.inputData.replace(/([\\;])/g, "\\$1");
>+        var parameters = e.inputData.match(/\S+/g);
>+        if (!parameters)
>+            parameters = [];
>+
>+        // replace in the command line.
>+        var expr = [SIMPLE_REPLACE, CUMUL_REPLACE, RANGE_REPLACE, NICK_REPLACE,
>+                    RECIP_REPLACE, ALL_REPLACE].join("|");
>+        aliasLine = aliasLine.replace(new RegExp(expr, "gi"), replaceAll);
>+
>+        if (errorMsg)
>+        {
>+            switch (errorMsg)
>+            {
>+                case 1:

Could you use the constants here please.

>+                    display(getMsg(MSG_ERR_REQUIRED_NR_PARAM, 
>+                                   [maxParamsAsked - parameters.length, 
>+                                    maxParamsAsked]), MT_ERROR);


>+        // return the revised command line.
>+        if (paramsUsed < parameters.length)
>+        {
>+            var pmstring = parameters.slice(paramsUsed, 
>+                                            parameters.length).join(" ");
>+            display(getMsg(MSG_EXTRA_PARAMS, pmstring), MT_WARN);
>+        }
>+        return aliasLine;
>+    }

Add a ";".


>+        
>+        var commandList;
>+        // Replace the variables if necessary:
>+        if (e.command.func.match(/\$\(.*\)|(?:^|[^\\])(?:\\\\)*;/))
>+            commandList = parseAlias(e.command.func, e);

Why do we need to call parseAlias if we have a ";" with an even number of "\"s
before it?


>+            else 
>+            {
>+                commandList[i] = commandList[i] + ";";
>+                commandList[i] = commandList[i].concat(commandList[i + 1]);

Are these two not the same as:

    commandList[i] = commandList[i] + ";" + commandList[i + 1];

>+                commandList.splice(i + 1, 1);
>+            }
>+        }
>+

Comment 7

[:Gijs (he/him)]

Attachment: even more style changes.

Yet more style changes.

Comment 8

[James Ross]

Comment on attachment 169460 [details] [diff] [review]
even more style changes.

You've tested enought of the built-in aliases to be sure they all work, right?

r=[e-mail] with that condition.

Comment 9

[:Gijs (he/him)]

All built-in aliases have been tested on Mozilla 1.7.3, and everything works
fine as far as I can see :-).

Comment 10

[James Ross]

Checked in --> FIXED.

This feature will be available in the next release of ChatZilla, 0.9.67.