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

Leverage Cow<'val, str> in scanner::Content #28

Closed
almann opened this issue Jun 1, 2021 · 1 comment · Fixed by #96
Closed

Leverage Cow<'val, str> in scanner::Content #28

almann opened this issue Jun 1, 2021 · 1 comment · Fixed by #96
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@almann
Copy link
Contributor

almann commented Jun 1, 2021

In #15, we added some new tokens that manipulate the text to be put into the content. We should leverage the borrowed form of the Cow when possible. There are at least three places where we should return the borrowed variant when there is no replacement:

/// Removes PartiQL escapes from a string literal and dequotes it.
#[inline]
fn normalize_string_lit(raw_text: &str) -> Cow<str> {
raw_text[1..(raw_text.len() - 1)].replace("''", "'").into()
}

/// Removes PartiQL escapes from a quoted identifier and dequotes it.
#[inline]
fn normalize_quoted_ident(raw_text: &str) -> Cow<str> {
raw_text[1..(raw_text.len() - 1)]
.replace(r#""""#, r#"""#)
.into()
}

Rule::Keyword => Content::Keyword(text.to_uppercase().into()),

@almann
Copy link
Contributor Author

almann commented Jun 1, 2021

As per @zslayton's original note in #15:

Not sure it's worth the dependency, but there's a small crate called cow-utils that provides cow_replace, cow_to_uppercase etc.

I am not sure it is worth the dependency as well, but something to definitely consider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants