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

Add Zipper.search_pattern/2 #154

Merged
merged 3 commits into from
Jun 28, 2024
Merged

Conversation

zachallaun
Copy link
Contributor

@zachallaun zachallaun commented Jun 28, 2024

Expanding on #147 which introduced Zipper.move_to_cursor/2, this PR adds a more general Zipper.search_pattern/2, which can match an arbitrarily nested pattern.

Also included is a small fix that makes do_find private (it was accidentally exposed) and a refactor for move_to_cursor.

cc @zachdaniel, @NickNeck

@NickNeck
Copy link
Contributor

I am not sure if I understand everything right, maybe I overlook something. But a pattern with __cursor__() returns the same result for move_to_cursor/2 and search_to_pattern/2 for the same zipper. So the generalisation of search_to_pattern/2 is that it doesn't needs a __cursor__(). If I am right with that assumption, I would suggest to rename search_to_pattern/2 in move_to_pattern/2 and ignore __cursor__() in this function. Then we have both functionalities with a clear name.

@zachallaun
Copy link
Contributor Author

So the generalisation of search_to_pattern/2 is that it doesn't needs a cursor().

Not quite; the generalization is that search_to_pattern/2 is that the pattern doesn't have to match at the top level, while move_to_cursor/2 does:

iex(1)> code = """
...(1)> if :foo != :bar do
...(1)>   IO.puts("hello")
...(1)> end
...(1)> """ |> Sourceror.parse_string!() |> Sourceror.Zipper.zip()

iex(2)> pattern = "IO.puts(__cursor__())"
"IO.puts(__cursor__())"

iex(3)> Sourceror.Zipper.move_to_cursor(code, pattern)
nil

iex(4)> Sourceror.Zipper.search_to_pattern(code, pattern)
#Sourceror.Zipper<
  #...
  {:__block__, [trailing_comments: [], leading_comments: [], delimiter: "\"", line: 2, column: 11],
   ["hello"]}
>

Now that I'm thinking about it, though, maybe find_pattern/2 might be a better name?

@NickNeck
Copy link
Contributor

@zachallaun thanks for your explanation. My assumption about how move_to_cursor works was wrong. Now everything is clear and looks great. I think find_pattern/2 would be a better name.

@doorgan
Copy link
Owner

doorgan commented Jun 28, 2024

This looks great!
On the naming part, I think search_pattern is a good name, find_* suggest to me that it will find with a function(like find, find_value, find_index etc in Enum), on the flip side I haven't seen many functions in the wild using search_* which makes me less worried about naming inconsistencies

@zachallaun zachallaun changed the title Add Zipper.search_to_pattern/2 Add Zipper.search_pattern/2 Jun 28, 2024
@zachallaun
Copy link
Contributor Author

Renamed search_to_pattern -> search_pattern 🙂

Copy link
Owner

@doorgan doorgan left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@doorgan doorgan merged commit 3e11732 into doorgan:main Jun 28, 2024
6 checks passed
@zachallaun zachallaun deleted the za-search-to-pattern branch June 28, 2024 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants