New revision of the patch attached. Comments inline: On Fri, Feb 26, 2016 at 8:16 AM, Marc Lehmann <[email protected]> wrote:
> On Fri, Feb 26, 2016 at 02:01:48AM -0800, Michael Stapelberg < > [email protected]> wrote: > > Thanks for the quick review. Find attached a new revision of the patch > > which addresses all your comments. > > Hi, thanks a lot for that! > > We ahve now looked at it in more detail, and will merge it, if you can work > on it a bit more. > > Here are the thoughts we came up with: > > 1. the coding style should match tghe coding style of the existing file - > right now, you enforce a different coding style in your changes. > I’m not used to programming without auto formatters anymore :) I’m using gofmt, clang-format and perltidy in other projects. Can you point me to documentation about the coding style that should be used, or point out the places where my code deviates from the coding style please? > > 2. We think the func: prefix is both a bit ugly, and a bit limited. How > about > implementing this syntax, # is a comment: > > the following two are for compatibility with existing syntax: > > string-without-$ # append match > string-with-$ # replace $n with matches > > # these would be new: > > shell:string # same as string-with-$ > perl:string # get's evaled > > in the former, only \$\d+ is recognised (as before), which rules out > using > $1 and so on in the shell string, but that is prpobably an o.k. > trade-off, > given that you can't quite them properly anywayws. > > the choice for perl: and it eval'ing trhe string would be because your > example could be easily embedded in the resources this way, and perl: as > opposed to eval: is similar to other areas of urxvt. > Done. > > which leaves the question if which variables should be available in the > eval string. I suggest these must be available (and documented): > > $self, $1, $2... > Do you have a suggestion as to how to make them available? (Ideally in the form of a diff to my attached patch revision.) > > While I think @_ should be set to the matches as well, so your func: > example could be written like this: > > perl: &urxvt::ext::inplace_vim::launch > > (and would arguably not cause eyedamange due to so many : and :: in > the same word :) > > I think you ran into some perl difficulties, and we thought about how to > fix > them: > > > - while ($text =~ /$matcher->[0]/g) { > > + while (my @captured = ($text =~ /\G.*?$matcher->[0]/)) { > > This would include \G.* in the match, and is thus buggy. This should work: > > > + while (my @captured = ($text =~ /\G.*?\K$matcher->[0]/)) { > > (i.e. add \K in between). > Done. > > > + pos($text) = $+[0]; > > Why is this needed? pos setting and \G "should" work in list context, or > did you have an issue with it not working? (I'm not an expert at list > context > matches). > pos is only advanced when using /g, which I couldn’t do anymore. Removing the line which calls pos() results in urxvt looping endlessly. > > > - if ($launcher !~ /\$/) { > > these matches would destroy the regex match variables, you should put these > But this line of code is unchanged — I don’t understand how my patch changes the behavior here. Can you clarify please? > into a function such as "launcher_type", which would return something like > "append" (string-without-$), "shell" (string-$, shell:), "perl" (perl:), > and > the remaining string. > > the perl: eval could then be: > > local @_ = @captured; > eval $launcher; > warn $@ if $@; > > or something like that. > > other solutions might be possible, but that would do the job and imho be an > improvement. > > this way, one could also get around the limitations with shell quoting, > e.g.: > > *.launcher.1: perl: $self->exec_async ("mycmd", $1, $2) > > What do you think? > > -- > The choice of a Deliantra, the free code+content > MORPG > -----==- _GNU_ http://www.deliantra.net > ----==-- _ generation > ---==---(_)__ __ ____ __ Marc Lehmann > --==---/ / _ \/ // /\ \/ / [email protected] > -=====/_/_//_/\_,_/ /_/\_\ >
From ce5a3ad83241c300809538fd5740dd4f9d62141a Mon Sep 17 00:00:00 2001 From: Michael Stapelberg <[email protected]> Date: Tue, 23 Feb 2016 21:17:34 +0100 Subject: [PATCH] matcher: allow executing functions as launcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this patch, users can specify perl strings to be evaluated, prefixed by “perl:”, e.g.: URxvt.matcher.button: 2 URxvt.matcher.pattern.1: ^([^ ]+):(\\d+) URxvt.matcher.launcher.1: perl:&urxvt::ext::inplace_vim::launch When launching, instead of forking and exec()ing, urxvt will call the specified function instead. In the example above, a separate extension called inplace_vim defines the following function: sub launch { my ($self, $file, $line) = @_; $self->tt_write($self->locale_encode("vim $file +$line\n")); } With that configuration, middle-clicking on a “filename:line” pair (such as output by grep -n) will simulate typing “vim filename +line\n”. --- src/perl/matcher | 51 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/src/perl/matcher b/src/perl/matcher index d991d68..b281949 100644 --- a/src/perl/matcher +++ b/src/perl/matcher @@ -74,6 +74,10 @@ Copy the current match to the clipboard. It is also possible to cycle through the matches using a key combination bound to the C<matcher:select> action. +When the special "perl" prefix is used in the C<matcher.launcher> resource, +the resource will be interpreted as a Perl string to eval rather than a +command to execute. + Example: load and use the matcher extension with defaults. URxvt.perl-ext: default,matcher @@ -88,6 +92,20 @@ Example: use a custom configuration. URxvt.matcher.pattern.2: \\B(/\\S+?):(\\d+)(?=:|$) URxvt.matcher.launcher.2: gvim +$2 $1 +Example: call a custom function + + URxvt.perl-ext: matcher,inplace_vim + URxvt.matcher.button: 2 + URxvt.matcher.pattern.1: ^([^ ]+):(\\d+) + URxvt.matcher.launcher.1: perl:&urxvt::ext::inplace_vim::launch + +Where the "inplace_vim" extension is defined like this: + + sub launch { + my ($self, $file, $line) = @_; + $self->tt_write($self->locale_encode("vim $file +$line\n")); + } + =cut my $url = @@ -100,6 +118,17 @@ my $url = )+ }x; +sub eval_or_exec { + my ($self, $type, @rest) = @_; + if ($type eq 'perl') { + my ($func, $match, @captured) = @rest; + @_ = ($self, @captured); + eval $func; + } else { + return $self->exec_async(@rest); + } +} + sub matchlist_key_press { my ($self, $event, $keysym, $octets) = @_; @@ -109,7 +138,7 @@ sub matchlist_key_press { my $i = ($keysym == 96 ? 0 : $keysym - 48); if ($i >= 0 && $i < @{ $self->{matches} }) { my @exec = @{ $self->{matches}[$i] }; - $self->exec_async (@exec[5 .. $#exec]); + eval_or_exec($self, @exec[5 .. $#exec]); } 1 @@ -203,7 +232,7 @@ sub most_recent { $row = $line->beg - 1; } if(@exec) { - return $self->exec_async (@exec); + return eval_or_exec($self, @exec); } () } @@ -309,21 +338,25 @@ sub find_matches { my @matches; for my $matcher (@{$self->{matchers}}) { my $launcher = $matcher->[1] || $self->{launcher}; - while ($text =~ /$matcher->[0]/g) { + while (my @captured = ($text =~ /\G.*?\K$matcher->[0]/)) { my $match = substr $text, $-[0], $+[0] - $-[0]; my @begin = @-; my @end = @+; my @exec; + pos($text) = $+[0]; if (!defined($off) || ($-[0] <= $off && $+[0] >= $off)) { - if ($launcher !~ /\$/) { - @exec = ($launcher, $match); + if (substr($launcher, 0, 5) eq 'perl:') { + @exec = ('perl', substr($launcher, 5), $match, @captured); + } elsif ($launcher !~ /\$/) { + @exec = ('exec', $launcher, $match); } else { + $launcher =~ s/^shell://; # It'd be nice to just access a list like ($&,$1,$2...), # but alas, m//g behaves differently in list context. - @exec = map { s/\$(\d+)|\$\{(\d+)\}/ + @exec = ('exec', map { s/\$(\d+)|\$\{(\d+)\}/ substr $text, $begin[$1 || $2], $end[$1 || $2] - $begin[$1 || $2] - /egx; $_ } split /\s+/, $launcher; + /egx; $_ } split /\s+/, $launcher); } push @matches, [ $line->coord_of ($begin[0]), $line->coord_of ($end[0]), $match, @exec ]; @@ -376,7 +409,7 @@ sub on_button_release { && join("\x00", @$cmd) eq join("\x00", $self->command_for($row,$col))) { if($self->valid_button($event)) { - $self->exec_async (@$cmd); + eval_or_exec($self, @$cmd); } } @@ -455,7 +488,7 @@ sub select_key_press { if ($keysym == 0xff0d || $keysym == 0xff8d) { # enter if ($self->{matches}) { my @match = @{ $self->{matches}[$self->{id}] }; - $self->exec_async (@match[5 .. $#match]); + eval_or_exec($self, @match[5 .. $#match]); } $self->select_leave; } elsif ($keysym == 0x79) { # y -- 2.7.0.rc3.207.g0ac5344
_______________________________________________ rxvt-unicode mailing list [email protected] http://lists.schmorp.de/mailman/listinfo/rxvt-unicode
