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

Document named captures #320

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

Earlopain
Copy link
Contributor

It's a nifty feature currently not mentioned in the docs. Hash keys can be interpolated which would then not make this match anymore but for demonstration purposes this is the best example I could come up with.

cc @dvandersluis

Unrelated to this, I've added ruby code styling to a code block that was previously missing it.

@dvandersluis
Copy link
Member

dvandersluis commented Oct 16, 2024

Something to point out is that you can do this without actually making them captures, ie

(pair
  (_ _key)
  (_ _key))

will also work to ensure the same value in both spots, without capturing _key.

Also (as discussed on the discord), this also works: ..._foo (multiple nodes as a named variable which can be referenced -- captures don't seem to work properly though!) . @marcandre this isn't documented or tested and I'm not sure if it's purposeful?

@Earlopain
Copy link
Contributor Author

without capturing _key

Oh, interesting. It's probably better to move it to https://docs.rubocop.org/rubocop-ast/node_pattern.html#_-for-any-single-node then. If the ..._name pattern is intentionally supported it'll not be far of from where it was introduced.

@Earlopain Earlopain force-pushed the docs-named-captures-reference branch 2 times, most recently from fc03d02 to 9dfbf8c Compare October 16, 2024 16:18
@Earlopain Earlopain force-pushed the docs-named-captures-reference branch from 9dfbf8c to 84af31f Compare October 16, 2024 16:20
@Earlopain Earlopain force-pushed the docs-named-captures-reference branch from 84af31f to 3f4db4c Compare October 16, 2024 16:21
Copy link
Member

@dvandersluis dvandersluis left a comment

Choose a reason for hiding this comment

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

Looks really good to me! Thanks @Earlopain

@marcandre
Copy link
Contributor

Something to point out is that you can do this without actually making them captures, ie

(pair
  (_ _key)
  (_ _key))

will also work to ensure the same value in both spots, without capturing _key.

Correct. Thanks for the PR 🚀

Also (as discussed on the discord), this also works: ..._foo (multiple nodes as a named variable which can be referenced -- captures don't seem to work properly though!) . @marcandre this isn't documented or tested and I'm not sure if it's purposeful?

That's incorrect. The lexer is often not space sensitive. In this case, ..._foo is actually treated the same as ... _foo, so _foo refers to a single last element (which must exist for a match), not to the variable number of remaining elements (which could be 0).

Would it be helpful to support named "rest"? I think it should look like ...name and $...name. I'd have to modify a bit the lexer and the parser, and maybe compiler to, not sure. If so, we can create a separate issue for the feature request; I'll close this one.

@marcandre marcandre merged commit cbd766e into rubocop:master Oct 16, 2024
19 checks passed
@marcandre
Copy link
Contributor

Note: I'm not too sure how to force the update of the docs, but I'm planning on resolving #319 soonish and will release that.

@dvandersluis
Copy link
Member

That's incorrect. The lexer is often not space sensitive. In this case, ..._foo is actually treated the same as ... _foo, so _foo refers to a single last element (which must exist for a match), not to the variable number of remaining elements (which could be 0).

Would it be helpful to support named "rest"? I think it should look like ...name and $...name. I'd have to modify a bit the lexer and the parser, and maybe compiler to, not sure. If so, we can create a separate issue for the feature request; I'll close this one.

Got it, that makes sense. I was pointing it out as an observation I had when playing with the pattern tester, but I didn't look into it closer. Thanks for the explanation.

I think it'd make sense in the abstract to support ...name to go with _name but I don't know if there's a specific requirement for it.

@marcandre
Copy link
Contributor

I think it'd make sense in the abstract to support ...name to go with _name but I don't know if there's a specific requirement for it.

Ok cool. Maybe we could mention in the docs that it's not supported but to open an issue if there's ever an actual use case?

@Earlopain Earlopain deleted the docs-named-captures-reference branch October 17, 2024 06:42
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.

3 participants