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

Testing the contextGenerator class #394

Merged
merged 8 commits into from
Jan 6, 2023

Conversation

iifawzi
Copy link
Contributor

@iifawzi iifawzi commented Nov 27, 2022

Signed-off-by: iifawzi iifawzie@gmail.com

Signed-off-by: iifawzi <iifawzie@gmail.com>
Signed-off-by: iifawzi <iifawzie@gmail.com>
Signed-off-by: iifawzi <iifawzie@gmail.com>
@codecov
Copy link

codecov bot commented Nov 27, 2022

Codecov Report

Base: 95.60% // Head: 97.40% // Increases project coverage by +1.79% 🎉

Coverage data is based on head (e1fc834) compared to base (a70dec0).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #394      +/-   ##
============================================
+ Coverage     95.60%   97.40%   +1.79%     
  Complexity     2174     2174              
============================================
  Files            68       68              
  Lines          4617     4617              
============================================
+ Hits           4414     4497      +83     
+ Misses          203      120      -83     
Impacted Files Coverage Δ
src/Tools/ContextGenerator.php 76.85% <0.00%> (+76.85%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: iifawzi <iifawzie@gmail.com>
Signed-off-by: iifawzi <iifawzie@gmail.com>
Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi
Copy link
Contributor Author

iifawzi commented Nov 27, 2022

can't find a proper name to test this? could you help @williamdes

if (preg_match('/([^[0-9]*)([0-9]*)/', $name, $parts) === false) {
return $name;
}

…d multiple occurrence of a keyword

Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi
Copy link
Contributor Author

iifawzi commented Nov 27, 2022

Most probably, this's a dead code

$len = strlen($words[$i]);
if ($len === 0) {
continue;
}

it checks whether the keyword is of length zero? how could this be possible?

Signed-off-by: iifawzi <iifawzie@gmail.com>
Comment on lines +2 to +5
RESERVED2 (R)
RESERVED3 (R)
RESERVED4 (R)
RESERVED5 (R)
Copy link
Contributor Author

@iifawzi iifawzi Nov 27, 2022

Choose a reason for hiding this comment

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

Multiple keywords are added here to test that the keywords will be in a new line

@williamdes
Copy link
Member

Hi,

Thank you for this, I will have a look when I finish my refactoring upstream at mariadb-mysql-kbs. I am near to have re-implementing the scraping into Rust code. Next should follow data for keywords and if possible functions and everything else.

I was thinking it would be nice to:

  • reduce the format of context files to a minimum with sections like:
    • Added
    • Removed
    • Changed

When using a vscode plugin to have the diff between files I did see that most of the file remains the same
It makes it uneasy to track changes and see missing elements.
We should rather build a diff system because it's real world about what is exactly the case.

Common is version v0 and MariaDB is v1 and so on and a version is nothing without the previous one. So it's like git.
If the contexts end up cleaned up like that we can fire up my upstream changes and ask for version x.y.z what are the known keywords and compare the lists to find missing on wrong ones.
Ideally when upstream and the parser are reconciled we can (maybe?) delete the context files and use my lib as source of truth. And still make manual updates of contexts and keywords

@williamdes williamdes self-assigned this Jan 6, 2023
@williamdes williamdes added this to the 5.7.0 milestone Jan 6, 2023
@williamdes williamdes merged commit cca5eb8 into phpmyadmin:master Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants