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

Deprecate matchall #26071

Merged
merged 1 commit into from
Feb 22, 2018
Merged

Deprecate matchall #26071

merged 1 commit into from
Feb 22, 2018

Conversation

nalimilan
Copy link
Member

It is not obvious that returning matching SubStrings rather than RegexMatch objects is a good idea.

Fixes #26049.

@nalimilan nalimilan added the search & find The find* family of functions label Feb 15, 2018
@@ -201,7 +201,7 @@ function complete_path(path::AbstractString, pos; use_envpath=false)
end

matchList = String[replace(s, r"\s" => "\\ ") for s in matches]
startpos = pos - lastindex(prefix) + 1 - length(matchall(r" ", prefix))
startpos = pos - lastindex(prefix) + 1 - count(c -> c == ' ', prefix)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this change is correct. AFAICT it is, but the original code is such a convoluted way of doing this that I feel like I may be missing something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right to me, too. Could be equalto(' ').

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I realized that right after pushing. Fixed while rebasing.

@mbauman mbauman added the deprecation This change introduces or involves a deprecation label Feb 15, 2018
@nalimilan
Copy link
Member Author

I'll merge once CI passes barring objections.

test/regex.jl Outdated
@test f(r"GCG","GCGCG") == ["GCG"]
@test f(r"GCG","GCGCG",overlap=true) == ["GCG","GCG"]
end
@test collect_eachmatch(r"a?b?", "asbd") == ["a","","b","",""] == f(r"""a?b?""", "asbd")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've got an old f() call in here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed!

It is not obvious that returning matching SubStrings rather than RegexMatch objects is a good idea.
@nalimilan nalimilan merged commit da98033 into master Feb 22, 2018
@nalimilan nalimilan deleted the nl/matchall branch February 22, 2018 09:10
@diam
Copy link

diam commented Jul 6, 2018

Ruby has a scan String method for the julia old matchall expected behavior.
https://ruby-doc.org/core-2.4.0/String.html#method-i-scan

Is it too late to add (or reserve) this name for julia-07?

words = collect(m.match for m in eachmatch(r"\d+(\.\d+)?", txt))

I'd much more prefer:

words = scan(r"\d+(\.\d+)?", txt)

-- Maurice

@StefanKarpinski
Copy link
Member

There's no need to reserve names: exporting new names from Base is not considered breaking.

@diam
Copy link

diam commented Nov 27, 2019

Bonjour,

This post is moved to discourse:
see https://discourse.julialang.org/t/best-way-to-get-all-substrings-or-numbers-matching-a-regex/31571

So what is the best way from julia-1.3+ for getting a String Vector from a regex like with matchall?

# findall?
...
# eachmatch?
...

-- Maurice

@KristofferC
Copy link
Member

Instead of commenting on old issues, it is better to open a new thread at https://discourse.julialang.org/ for questions.

@kubark42
Copy link

Instead of commenting on old issues, it is better to open a new thread at https://discourse.julialang.org/ for questions.

I'd like to lightly disagree with this statement. I found this issue by using the changelog. I was looking for why matchall() no longer worked, and to understand what its replacement was. Without @diam's question and subsequent link, I would never have found the discourse page.

Of course, the true value is the cross-link to the discourse page, but since that doesn't happen automatically it's valuable to see discourse open here before ultimately moving to a better home.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation search & find The find* family of functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected behaviour in matchall
6 participants