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 escaping nested close brackets when parsing layout renderers #3260

Merged

Conversation

lobster2012-user
Copy link
Contributor

@lobster2012-user lobster2012-user commented Mar 30, 2019

fixes #3193

@codecov
Copy link

codecov bot commented Mar 30, 2019

Codecov Report

❗ No coverage uploaded for pull request base (dev@a095392). Click here to learn what that means.
The diff coverage is 100%.

@@          Coverage Diff          @@
##             dev   #3260   +/-   ##
=====================================
  Coverage       ?     80%           
=====================================
  Files          ?     355           
  Lines          ?   27986           
  Branches       ?    3727           
=====================================
  Hits           ?   22310           
  Misses         ?    4602           
  Partials       ?    1074

Copy link
Member

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

[InlineData(@" ${cached:${cached:${literal:text={0\} {1\}}}}")]
[InlineData(@" ${cached:${cached:${cached:${literal:text={0\} {1\}}}}}")]
[InlineData(@"${cached:${cached:${cached:${cached:${literal:text={0\} {1\}}}}}}")]
public void Issue_3193_Nested_Сlosing_Braces(string input)
Copy link
Member

Choose a reason for hiding this comment

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

Hi! Thanks for the included tests!

Unfortunately it's a bit hard to follow. We are trying to use AAA in unit tests,see https://www.c-sharpcorner.com/UploadFile/dacca2/fundamental-of-unit-testing-understand-aaa-in-unit-testing/

}
else
{
throw new Xunit.Sdk.XunitException("NOT SUPPORTED");
Copy link
Member

Choose a reason for hiding this comment

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

Is this assert.Fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@304NotModified
Copy link
Member

Any idea if this will fix also #2844

@lobster2012-user
Copy link
Contributor Author

lobster2012-user commented Mar 31, 2019

@pablinos needed to use double backslashes. I would not say that there is a problem at all. If he had more nested renders, then yes, my pr would fix his problem.
I would close issue #2844.

@pablinos
Copy link

Oooo, this is exciting, I'll have to try this out. I never properly solved the problem I was having, but I notice a few other items referencing the issue #2844 and as @lobster2012-user can't reproduce it, it may well be that it's been fixed.

I'll have another try and let you know.

@304NotModified 304NotModified added the bug Bug report / Bug fix label Mar 31, 2019
@lobster2012-user
Copy link
Contributor Author

(I moved comment)
I updated test.
I think, whether to move the code in SimpleLayoutTests.cs.

@304NotModified
Copy link
Member

I think, whether to move the code in SimpleLayoutTests.cs.

I think that's better, as the test is "acting" on SimpleLayout.

Copy link
Member

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

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

Great work! Thanks!

@304NotModified 304NotModified added this to the 4.6.2 milestone Mar 31, 2019
@304NotModified 304NotModified changed the title Issue 3193 nested closing braces Fix escaping nested close brackets Apr 1, 2019
@304NotModified 304NotModified merged commit a3ce8ae into NLog:dev Apr 1, 2019
@304NotModified 304NotModified changed the title Fix escaping nested close brackets Fix escaping nested close brackets when parsing layout renderers Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report / Bug fix nlog-configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using {{}} in config
3 participants