Skip to content

Fixing conflict in lexer when '*/*' was in a query. #287

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

Merged
merged 3 commits into from
Feb 10, 2020

Conversation

niconoe-
Copy link
Contributor

@niconoe- niconoe- commented Feb 2, 2020

This is willing to fix #285

@codecov
Copy link

codecov bot commented Feb 2, 2020

Codecov Report

Merging #287 into QA will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##               QA   #287   +/-   ##
=====================================
  Coverage     100%   100%           
- Complexity   1865   1867    +2     
=====================================
  Files          63     63           
  Lines        4526   4530    +4     
=====================================
+ Hits         4526   4530    +4

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

Perfect, could you add more tests?

@niconoe-
Copy link
Contributor Author

niconoe- commented Feb 2, 2020

Perfect, could you add more tests?

Sure, but what kind of tests do you want me to provide? I already tested all valid use cases of */* I know.

@williamdes
Copy link
Member

Perfect, could you add more tests?

Sure, but what kind of tests do you want me to provide? I already tested all valid use cases of */* I know.

Maybe a new line, multiples wildcards
What would be the behavior of a column named like a wildcard? backquoted vs not backquoted

I would like more tests on strange queries more that normal ones if possible

@niconoe-
Copy link
Contributor Author

niconoe- commented Feb 2, 2020

Ok I'll provide that. Thank you for your feedback.

@niconoe-
Copy link
Contributor Author

niconoe- commented Feb 2, 2020

I added more complex test cases in the provider of the lexer. Is this OK now?
Thanks a lot.

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

💯 thank you!

Okay now, nothing more to be made before merge IMO

@williamdes williamdes changed the base branch from master to QA February 10, 2020 19:26
@williamdes williamdes closed this Feb 10, 2020
@williamdes williamdes reopened this Feb 10, 2020
@williamdes
Copy link
Member

CI passed ✔️

williamdes added a commit that referenced this pull request Feb 10, 2020
…query.

Pull-request: #287
Fixes: #285

Signed-off-by: William Desportes <[email protected]>
@williamdes williamdes merged commit 101e0b0 into phpmyadmin:QA Feb 10, 2020
@williamdes williamdes self-assigned this Feb 10, 2020
@williamdes
Copy link
Member

Merged, thank you @niconoe- !

@williamdes williamdes added this to the 4.5.1 milestone Feb 11, 2020
@niconoe- niconoe- deleted the fix-issue-285 branch February 27, 2023 14:51
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.

Parsing comments issues
2 participants