Add name capture group support for regex triggers by Delwing · Pull Request #4731 · Mudlet/Mudlet

Delwing · 2021-02-01T22:02:09Z

Brief overview of PR changes/additions

It populates matches with named capture groups:
https://www.regular-expressions.info/named.html

Some skeletal retrieval was there, but never went any further.

I've tested that on regexp trigger and tempRegexTrigger - I guess only those 2 need support for that.
Multimatches also are made to work.

Please take a special look at the retrieval code TTrigger.ccp:352

Motivation for adding to Mudlet

Multiple triggers matching similar thing can be hell if searched needle is swapping place.
Named groups are very convinent.

Other info (issues closed, discussion etc)

Quick test regexp

if demoRegex then
  killTrigger(demoRegex)
end
demoRegex = tempRegexTrigger("^My name is (?<name>\\w+)\\.$", function() display(matches) cecho("\n<red>".. matches.name) end)
feedTriggers("\nMy name is Dargoth.\n")
echo("\n")

add-deployment-links · 2021-02-01T22:02:59Z

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

SlySven

I like the idea of this - not sure I can get my head around it at the moment though...

😵

src/TLuaInterpreter.cpp

SlySven · 2021-02-03T19:13:07Z

src/TTrigger.cpp

                matchStatePair.second->conditionMatched();
                matchStatePair.second->multiCaptureList.push_back(captureList);
                matchStatePair.second->multiCapturePosList.push_back(posList);
+                if (nameMatches != nullptr) {


The != nullptr is unneeded...

Will remove it and test 👍🏻

vadi2

Works well 👍

vadi2 · 2021-02-02T07:52:00Z

src/TTrigger.cpp

-
        tabptr = name_table;
        for (i = 0; i < namecount; i++) {
+            int n = (tabptr[0] << 8) | tabptr[1];


Could you add a comment for this construction?

Well... I can reference snippet that I've used in comment + my (hopefully) correct understanding of that.

That would work

vadi2 · 2021-02-03T17:53:42Z

src/TLuaInterpreter.cpp

@@ -13653,10 +13653,11 @@ std::pair<bool, QString> TLuaInterpreter::discordApiEnabled(lua_State* L, bool w
 }



In order for people to be able to check in their scripts if the player is running a Mudlet that supports this - add namedmatches = true to mudlet.supports, similar to how coroutines are there already.

See also https://wiki.mudlet.org/w/Manual:Advanced_Lua#Coroutines

vadi2 · 2021-02-03T19:10:44Z

src/TTrigger.cpp

+        NameGroupMatches* ng = &nameGroups;
+        updateMultistates(regexNumber, captureList, posList, ng);


This would work just as well:

Suggested change

NameGroupMatches* ng = &nameGroups;

updateMultistates(regexNumber, captureList, posList, ng);

updateMultistates(regexNumber, captureList, posList, &nameGroups);

src/TLuaInterpreter.cpp

SlySven · 2021-02-03T19:22:05Z

src/TTrigger.h

 #define REGEX_PROMPT 7
 #define MAX_CAPTURE_GROUPS 33

+using NameGroupMatches = QVector<QPair<QString, QString>>;


🤯 This is a new concept for me - anyone else not get it? https://stackoverflow.com/q/20790932/4805858 might help.

Check out https://www.internalpointers.com/post/differences-between-using-and-typedef-modern-c as well

I wanted to have this generics more comprehensible.

SlySven · 2021-02-03T19:57:24Z

src/TTrigger.cpp

        tabptr = name_table;
        for (i = 0; i < namecount; i++) {
+            int n = (tabptr[0] << 8) | tabptr[1];
+            auto name = QString::asprintf("%*s", name_entry_size - 3, tabptr + 2).trimmed();


Qt documentation says:

Warning: We do not recommend using QString::asprintf() in new Qt code. Instead, consider using QTextStream or arg(), both of which support Unicode strings seamlessly and are type-safe. Here's an example that uses QTextStream:

QString result;
QTextStream(&result) << "pi = " << 3.14;
// result == "pi = 3.14"
For translations, especially if the strings contains more than one escape sequence, you should consider using the arg() function instead. This allows the order of the replacements to be controlled by the translator.

This function was introduced in Qt 5.5.

See also arg().

Yes! I've read it, and was pretty aware of that. arg() was not alternative in this case, not accepting range I guess... My understanding of this part is a bit limited.
It is based on:
https://github.com/vmg/pcre/blob/master/pcredemo.c

I guess I've found solution not to use asprintf

SlySven · 2021-02-04T01:19:57Z

👍 You know named capture groups is a great way to simplify multiple trigger items with different numbers of arguments and them not being in the same position in different ones...! Something that really should be mentioned in the release announcement.

vadi2 · 2021-02-04T06:51:08Z

src/TTrigger.cpp

-
        tabptr = name_table;
        for (i = 0; i < namecount; i++) {
+            int n = (tabptr[0] << 8) | tabptr[1];


That would work

Delwing · 2021-02-04T07:19:09Z

That was my main motivation - same handler function, different argument order :) Had just use case like that, so started digging :)

vadi2 · 2021-02-04T07:25:28Z

Include that in the docs for sure as a practical example!

Delwing · 2021-02-04T07:32:21Z

Yep! Will do.

Co-authored-by: Piotr Wilczynski <piotr.wilczynski@bisnode.com>

add name capture group support

393f969

Delwing requested a review from a team as a code owner February 1, 2021 22:02

Delwing requested review from a team February 1, 2021 22:02

SlySven reviewed Feb 2, 2021

View reviewed changes

remove reduntant line

9f0142d

SlySven reviewed Feb 3, 2021

View reviewed changes

src/TLuaInterpreter.cpp Show resolved Hide resolved

SlySven reviewed Feb 3, 2021

View reviewed changes

vadi2 reviewed Feb 3, 2021

View reviewed changes

SlySven reviewed Feb 3, 2021

View reviewed changes

Piotr Wilczynski added 2 commits February 4, 2021 01:02

cr review corrections

d4de791

remove not needed nullptr check

6cf2dab

vadi2 approved these changes Feb 4, 2021

View reviewed changes

Delwing changed the title ~~add name capture group support~~ Add name capture group support for regex triggers Feb 4, 2021

Delwing merged commit 27d4785 into Mudlet:development Feb 7, 2021

Chris7 pushed a commit to Chris7/Mudlet that referenced this pull request Jan 2, 2022

Add name capture group support for regex triggers (Mudlet#4731)

f89196e

Co-authored-by: Piotr Wilczynski <piotr.wilczynski@bisnode.com>

		@@ -13653,10 +13653,11 @@ std::pair<bool, QString> TLuaInterpreter::discordApiEnabled(lua_State* L, bool w
		}

		NameGroupMatches* ng = &nameGroups;
		updateMultistates(regexNumber, captureList, posList, ng);

Uh oh!

Conversation

Delwing commented Feb 1, 2021

Brief overview of PR changes/additions

Motivation for adding to Mudlet

Other info (issues closed, discussion etc)

Uh oh!

add-deployment-links bot commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test versions

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vadi2 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlySven commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Delwing commented Feb 4, 2021

Uh oh!

vadi2 commented Feb 4, 2021

Uh oh!

Delwing commented Feb 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

add-deployment-links bot commented Feb 1, 2021 •

edited

Loading

vadi2 left a comment •

edited

Loading

SlySven commented Feb 4, 2021 •

edited

Loading