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

Name collision #11

Open
jlaurens opened this issue Jul 11, 2024 · 8 comments
Open

Name collision #11

jlaurens opened this issue Jul 11, 2024 · 8 comments

Comments

@jlaurens
Copy link
Contributor

Pattern is a general name also used by some frameworks.
lpeglib.Pattern seems better than just lpeg.Pattern.

@Josef-Friedrich
Copy link
Collaborator

I would like to keep the lpeg prefix. What name would you want to use instead of Pattern?

@jlaurens
Copy link
Contributor Author

I have the impression that keeping the lpeg prefix might not be a good idea because the lpeg namespace is not de facto free. After some tests, it happens that for my own usage, the distinction between Pattern and Capture does not help that much. So I have patched things for my own usage such that you don't need to make any change. I use lpeglib.Pattern to mimic builtin oslib, iolib`...

@Josef-Friedrich
Copy link
Collaborator

Josef-Friedrich commented Jul 12, 2024

In this repo the lpeg table is local

local lpeg = {}

The name of the table should therefore have no effect on code outside.

In the LuaTeX repo lpeg is global

https://github.com/LuaCATS/tex-luatex/blob/4c2c8fd3bbc31cf8c0af2b6389ca598d2a935f52/library/lpeg.lua#L48

So I just had to delete the local keyword and was able to synchronize the two type definitions: The type defintions for the
upstream LPeg project and the type defintions for the embedded LPeg (and outdated version) in LuaTeX

@Josef-Friedrich
Copy link
Collaborator

Josef-Friedrich commented Jul 12, 2024

If I remember correctly, the distinction between Pattern and Capture was introduced by @AndreasMatthias. May be he can help out?

@AndreasMatthias
Copy link
Contributor

AFAIK types Pattern and Capture existed since the beginning. They were not introduced by me. But what I changed then, was making Capture an alias for Pattern such that LuaLS knows how these two types relate to each other. Without this alias some operations had an undefined result type if I remember correctly.

Whether or not aliases are expanded by LuaLS depends on the configuration setting hover.expandAlias.
See https://github.com/LuaCATS/lpeg/blob/main/README.md and https://luals.github.io/wiki/settings/#hoverexpandalias.

I usually disable hover.expandAlias since I prefer to be reminded whether I'm dealing with a Capture or a Pattern.

@jlaurens
Copy link
Contributor Author

I find the distinction between Capture and Pattern quite blurry

  • lpeg.P(fun) → Pattern but it is equivalent to lpeg.Cmt which is a Capture
  • lpeg.C(1)^1^-1 → Pattern whereas it is lpeg.C(1) which is a Capture
  • lpeg.P(lpeg.C(1))→ Pattern whereas it is also lpeg.C(1)

In general, if Capture is similar to a "pattern with captures" then probably some additional Captures should be recognized.

The only interest I see in Capture is that lpeg.B should no accept Capture. In that case, the Capture recognition seems very loose.
Something else?

@Josef-Friedrich
Copy link
Collaborator

@AndreasMatthias Thank you for the clarification!

@Josef-Friedrich
Copy link
Collaborator

@jlaurens
If I understand you correctly, you would merge Pattern and Capture under a new more specific name (PatternWithCapture)?
How would you handle the lpeg.B corner case? That would be a major API breaking change? Than we should put up a warning in the README or in the type defintions?

Feel free to submit a PR.

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

No branches or pull requests

3 participants