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

string match broken with multiline input #7938

Closed
lelgenio opened this issue Apr 18, 2021 · 5 comments
Closed

string match broken with multiline input #7938

lelgenio opened this issue Apr 18, 2021 · 5 comments
Assignees
Milestone

Comments

@lelgenio
Copy link
Contributor

lelgenio commented Apr 18, 2021

My information

~> fish --version
fish, versão 3.2.1
~> echo $version
3.2.1
~> uname -a
Linux artix-i15 5.11.13-zen1-1-zen #1 ZEN SMP PREEMPT Sun, 11 Apr 2021 10:10:31 +0000 x86_64 GNU/Linux

How to reproduce

~> sh -c 'env HOME=$(mktemp -d) fish'
~> echo "100 101" | string match -ra '(?<nums>\d+)'
100
100
101
101
~> echo $nums # Ok
100 101
~> echo "100 101" | string match -qra '(?<nums>\d+)'
~> echo $nums # Ok
100 101
~> seq 100 101 | string match -ra '(?<nums>\d+)'
100
100
101
101
~> echo $nums # No output?

~> seq 100 101 | string match -qra '(?<nums>\d+)'
~> echo $nums # Only matches first?
100

I expected all of them to have the same result.

I bet there's no problem at all and i'm just missing something 🙄

@ridiculousfish
Copy link
Member

Nope it's real, well found!

@ridiculousfish ridiculousfish self-assigned this Apr 19, 2021
@ridiculousfish ridiculousfish added this to the fish 3.3.0 milestone Apr 19, 2021
@faho
Copy link
Member

faho commented Apr 19, 2021

There's a few things here:

  1. Variables are only imported for the first matching argument. We don't have lists of lists, and capturing groups can match multiple times, so anything else doesn't seem feasible. You demonstrated this yourself with the first example
  2. string handles input via stdin as one argument per line, so this only imports the matches for the first line.

This is currently documented:

It will create a variable with the name of the group, in the default scope, for each named capturing group, and set it to the value of the capturing group in the first matched argument.

I'm not sure there is better behavior here - how do you handle echo "100 101"\n"102 103"? Do you collapse it so $num becomes [100, 101, 102, 103]?

(tbh I've so far not found a great use case for variable importing to begin with, and this depends on what you want to do with it. I don't think we use it anywhere?)

@ridiculousfish
Copy link
Member

ridiculousfish commented Apr 20, 2021

The problematic case is seq 100 101 | string match -ra '(?<nums>\d+)'. It seems obvious that this should set $nums to something. I plan to keep it the first matching argument, so it will output 100, as documented.

@lelgenio
Copy link
Contributor Author

I don't understand why default to ignoring other arguments instead of letting the user be explicit by using $myvar[1].

@krobelus
Copy link
Member

@lelgenio the problem is that you can have something like

string match -qra '(?<nums>\d)' '1 2' '3 4'

so the array is already used to access all matches in the first argument (or input line).
Really depends on the use case which one is better.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants