-
Notifications
You must be signed in to change notification settings - Fork 280
Message handler: add quote_begin, quote_end commands #5696
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
base: develop
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #5696 +/- ##
===========================================
- Coverage 80.39% 80.38% -0.02%
===========================================
Files 1688 1688
Lines 207403 207485 +82
Branches 73 73
===========================================
+ Hits 166749 166781 +32
- Misses 40654 40704 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
32acae2
to
3604763
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good. It would be nice if this also had the implementation of this for XML so that it was clear why it was valueable and no-one will accidentally 'simplify' it out.
src/util/ui_message.h
Outdated
case 96: // bright_cyan | ||
return std::string(); | ||
case '<': // quote_begin | ||
return "<quote>"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this is a good idea as it changes the XML output format. I'd stick to single quotes for now and introduce a different format in a separate PR if desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now factored this into a commit of its own, which could thus easily be dropped from the series.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really prefer to drop this commit. IMO, it causes unnecessary hassle to XML output users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, just left behind a comment that this could be done.
402b16a
to
e423424
Compare
e423424
to
3bcd0c6
Compare
3bcd0c6
to
99e7c06
Compare
99e7c06
to
5b56934
Compare
5b56934
to
4429ed9
Compare
4429ed9
to
90be428
Compare
90be428
to
ff0b549
Compare
ff0b549
to
de2a0c5
Compare
de2a0c5
to
01e7746
Compare
4dcfcd6
to
fcacae2
Compare
These will permit custom formatting of quoting for each message handler, See diffblue#4875 for discussion.
This continues the work of diffblue#4875, but now using quote_begin/quote_end stream modifiers.
This is in the spirit of diffblue#4875, but now using quote_begin/quote_end stream modifiers.
fcacae2
to
476ebc2
Compare
These will permit custom formatting of quoting for each message handler, and for a start introduce tags for XML. See #4875 for discussion.
Please review commit-by-commit.