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

Fix the problem with inner double quotes #195

Merged
merged 12 commits into from
Aug 13, 2018

Conversation

OlegNadymov
Copy link
Contributor

@OlegNadymov OlegNadymov commented Aug 10, 2018

Do I correct create expression for my case with inner double quotes?
After move from System.Dynamic.Linq to System.Dynamic.Linq.Core all my similar expressions were broken

@OlegNadymov OlegNadymov changed the title This unit test shows problem with inner double quotes This unit test shows the problem with inner double quotes Aug 10, 2018
@OlegNadymov OlegNadymov changed the title This unit test shows the problem with inner double quotes Fix the problem with inner double quotes Aug 10, 2018
@StefH StefH self-requested a review August 10, 2018 10:53
Copy link
Collaborator

@StefH StefH left a comment

Choose a reason for hiding this comment

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

There are some merge conflicts, please merge main into this branch and fix merge conflicts.

Copy link
Collaborator

@StefH StefH left a comment

Choose a reason for hiding this comment

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

Small updates in code.

while (true)
{
int index2 = s.IndexOf(quote, index1);
if (index2 < 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use {} in this new code. Example:

if (index2 < 0)
{
    break;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok! I'm done.

if (index2 < 0)
break;
if (index2 + 1 < s.Length && s[index2 + 1] == quote)
s = s.Remove(index2, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use {} in this new code. Example:

if (index2 < 0)
{
    break;
}

Copy link
Contributor Author

@OlegNadymov OlegNadymov Aug 12, 2018

Choose a reason for hiding this comment

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

@StefH Ok! It was done.

@codecov
Copy link

codecov bot commented Aug 10, 2018

Codecov Report

Merging #195 into master will increase coverage by 0.25%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
+ Coverage   82.61%   82.86%   +0.25%     
==========================================
  Files          37       37              
  Lines        3658     3671      +13     
  Branches      493      495       +2     
==========================================
+ Hits         3022     3042      +20     
  Misses        498      498              
+ Partials      138      131       -7
Impacted Files Coverage Δ
...ystem.Linq.Dynamic.Core/Parser/ExpressionParser.cs 78.22% <100%> (+0.4%) ⬆️
src/System.Linq.Dynamic.Core/Parser/TypeHelper.cs 88.2% <0%> (+0.47%) ⬆️
...c/System.Linq.Dynamic.Core/Tokenizer/TextParser.cs 98.03% <0%> (+1.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22ef22c...e13c9d1. Read the comment docs.

@OlegNadymov
Copy link
Contributor Author

@StefH I made the required changes in the code. Something else needs to be done?

Copy link
Contributor Author

@OlegNadymov OlegNadymov left a comment

Choose a reason for hiding this comment

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

@StefH, all required changes were done

@StefH
Copy link
Collaborator

StefH commented Aug 12, 2018

Thanks a lot.

A small question, how does the code behave if you use something like an incomplete escaped string as a search query?
Example test \"abc.

@OlegNadymov
Copy link
Contributor Author

OlegNadymov commented Aug 12, 2018

@StefH I've just tried it. And it works is like before. It throws ParseError(_textPos, Res.UnterminatedStringLiteral);

@OlegNadymov
Copy link
Contributor Author

@StefH may be you skipped my comment about new code.

@StefH
Copy link
Collaborator

StefH commented Aug 13, 2018

Ok. I will merge the code soon.

@OlegNadymov
Copy link
Contributor Author

@StefH ok! Thank you! And update nuget please.

@StefH StefH merged commit 870eb29 into zzzprojects:master Aug 13, 2018
@OlegNadymov
Copy link
Contributor Author

@StefH did you update nuget?

@StefH
Copy link
Collaborator

StefH commented Aug 13, 2018

Not yet sorry. Tomorrow morning.

@StefH
Copy link
Collaborator

StefH commented Aug 14, 2018

NuGets uploaded

@OlegNadymov
Copy link
Contributor Author

@StefH thank you!!

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

Successfully merging this pull request may close these issues.

2 participants