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

SimpleStringReader: fix DebuggerDisplay value #3195

Conversation

lobster2012-user
Copy link
Contributor

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

fixes #3194

#if DEBUG
        internal static string BuildCurrentState(string done, char current, string todo)
            => $"done: '{done}'.   current: '{current}'.   todo: '{todo}'";
        internal string CurrentState
        {
            get
            {
                var current = (char)Peek();
                if (Position < 0 || Position > Text.Length)
                {
                    return BuildCurrentState(done: "INVALID_CURRENT_STATE", current: char.MaxValue, todo: "INVALID_CURRENT_STATE");
                }
                var done = Substring(0, Position);
                var todo = ((Position < _text.Length) ? Text.Substring(Position) : "");
                return BuildCurrentState(done: done, current: current, todo: todo);
            }
        }
#endif

added tests
@lobster2012-user
Copy link
Contributor Author

lobster2012-user commented Mar 23, 2019

Need to change the config * .yml,
The tests only work for the release configuration now.
It is necessary to run only debug tests as well
dotnet tests --filter displayName~Debug

@snakefoot
Copy link
Contributor

It is necessary to run only debug tests as well

Well the code is only activated when making DEBUG-builds. For release-builds then the code is non-existing.

@lobster2012-user
Copy link
Contributor Author

lobster2012-user commented Mar 23, 2019

That's right, but the test result is not obvious. At your discretion.
For dev branch you can also run debug tests.

@snakefoot
Copy link
Contributor

The AppVeyor-build runs the final code-coverage test using a DEBUG-build for proper symbol-information.

@304NotModified
Copy link
Member

304NotModified commented Mar 23, 2019

Thanks for the PR!

The build fails, see this error:

Missing headers (copy them form other another file).
1 errors:


-C:\projects\nlog\tests/NLog.UnitTests\SimpleStringReaderDebugViewTests.cs

Could you please fix this? Thanks 😊

(See also build log)

@codecov
Copy link

codecov bot commented Mar 24, 2019

Codecov Report

Merging #3195 into dev will increase coverage by <1%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##             dev   #3195    +/-   ##
======================================
+ Coverage     80%     80%   +<1%     
======================================
  Files        354     354            
  Lines      27909   27913     +4     
  Branches    3706    3707     +1     
======================================
+ Hits       22251   22271    +20     
+ Misses      4597    4579    -18     
- Partials    1061    1063     +2

@304NotModified 304NotModified changed the title issue_3194_simple_string_reader_debug_view SimpleStringReader: fix DebuggerDisplay value Mar 24, 2019
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, looks good, but please do some renames and a file move - see notes. Then I could merge it :)

tests/NLog.UnitTests/SimpleStringReaderDebugViewTests.cs Outdated Show resolved Hide resolved
tests/NLog.UnitTests/SimpleStringReaderDebugViewTests.cs Outdated Show resolved Hide resolved
@304NotModified 304NotModified added this to the 4.6.1 milestone Mar 24, 2019
@lobster2012-user
Copy link
Contributor Author

I fixed all and changed the logic of todo.

@304NotModified 304NotModified merged commit af3a22d into NLog:dev Mar 25, 2019
@304NotModified
Copy link
Member

Thanks! Merged! :)

@304NotModified 304NotModified added the enhancement Improvement on existing feature label Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement Improvement on existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SimpleStringReader.CurrentState
3 participants