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

Feature Request: pyformat param types #545

Closed
chooban opened this issue Dec 19, 2022 · 5 comments · Fixed by #582
Closed

Feature Request: pyformat param types #545

chooban opened this issue Dec 19, 2022 · 5 comments · Fixed by #582
Labels

Comments

@chooban
Copy link

chooban commented Dec 19, 2022

Describe the Feature
Would you consider adding support for pyformat param types? I've run a few of our snowsql scripts throught the formatter and for the most part it's absolutely fantastic, but it trips up on our param types which use, e.g. %(end_date)s.

I thought I should ask before even considering diving in and attempting a PR, in case it was something that had been previously considered and rejected.

Why do you want this feature?
This would increase the number of situations in which the tool could be used, and is expanding on an existing feature rather than adding something entirely new.

@nene
Copy link
Collaborator

nene commented Dec 19, 2022

It would be nice to support this kind of parameters, but I wouldn't really like to add a specific option for this Python formatting syntax. Rather I would consider providing a more flexible way of customizing the possible parameter types.

But I'm not fully sure what would be the best API for this. Currently the paramTypes setting categorizes parameters to 4 types:

  • positional (?)
  • numbered (like :1)
  • named (like :name)
  • quoted (like :"name")

This Python syntax looks most similar to the quoted type, with parenthesis sort-of used for quoting, plus there's this extra s code at the end (which I understand can be a lot more complex than just a single letter). But we can't simply add another quote type that would match (name)s part as that would most likely interfere with parsing of parenthesis pairs.

Assuming these pyformat parameters can be parsed with regular expressions, the syntax could be something like:

paramTypes: {
  custom: [
    { regex: String.raw`%\((\w+)\)[a-z]` }
  ]
}

Additional consideration is that we also need a way to determine the actual name inside the parenthesis. This is used by the parameter-substitution feature. A simplest approach would be to just use a regex capture group as I've done above.

@chooban
Copy link
Author

chooban commented Dec 20, 2022

That would definitely be good for us, though it sounds like a bit of a bigger rewrite of the param code, unless it's already using regex behind the scenes.

@nene
Copy link
Collaborator

nene commented Dec 20, 2022

Well, it is indeed using regexes behind the scenes. It's more about how the final regexes for matching parameter tokens get built.

@nene
Copy link
Collaborator

nene commented Mar 22, 2023

Related discussion: #551

@nene
Copy link
Collaborator

nene commented Mar 22, 2023

This is now implemented in 12.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants