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.

Sign up for GitHub

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

Jump to bottom

Improve exception error messages #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve exception error messages #109

wants to merge 1 commit into from

Conversation

muesli4
Copy link

@muesli4 muesli4 commented Feb 24, 2018

Proposed improvement to #108. Let me know what you think about it or if that's something you don't want to change.

@ilyapopov
Copy link

Maybe it is better to just derive from std::runtime_error. This way you get wgat() etc. for free.

@muesli4
Copy link
Author

muesli4 commented Sep 5, 2019 •
edited
Loading

Maybe it is better to just derive from std::runtime_error. This way you get wgat() etc. for free.

The thing is that the existing code is not really composable in a way that makes refactoring easy. Is C++11 allowed? If so I can make it work with std::runtime_error (because I require constructor delegation).

@muesli4
Copy link
Author

muesli4 commented Sep 5, 2019

I updated the exception classes to inherit from std::runtime_error. The examples seem to work and the tests pass. I can condense this into one commit.

@muesli4 muesli4 force-pushed the master branch 3 times, most recently from ec53ee7 to a31c20e Compare December 26, 2019 19:02
@muesli4
Copy link
Author

muesli4 commented Dec 26, 2019

I rebased my changes on the recent version and it is only one commit now.

@muesli4
Copy link
Author

muesli4 commented Dec 26, 2019

I deliberately caused some of the errors:

> mpd-touch-screen-gui
mpd-touch-screen-gui 0.0.1
Found configuration file: /home/moritz/.config/mpd-touch-screen-gui/program.conf
setting not found exception: program.[default_font]
> mpd-touch-screen-gui
mpd-touch-screen-gui 0.0.1
Found configuration file: /home/moritz/.config/mpd-touch-screen-gui/program.conf
setting type exception: program.default_font.path
> mpd-touch-screen-gui
mpd-touch-screen-gui 0.0.1
Found configuration file: /home/moritz/.config/mpd-touch-screen-gui/program.conf
parse exception: in /home/moritz/.config/mpd-touch-screen-gui/program.conf, at line 6: syntax error
> mpd-touch-screen-gui
mpd-touch-screen-gui 0.0.1
Found configuration file: /home/moritz/.config/mpd-touch-screen-gui/program.conf
parse exception: in /home/moritz/.config/mpd-touch-screen-gui/program.conf, at line 6: duplicate setting name

@muesli4
Copy link
Author

muesli4 commented Sep 5, 2020

Is there anything I can improve or change such that this will be merged?

hyperrealm
hyperrealm reviewed Oct 2, 2020
View reviewed changes
Copy link
Owner

@hyperrealm hyperrealm left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay. :-(

lib/libconfigcpp.c++ Outdated Show resolved Hide resolved
lib/libconfigcpp.c++ Outdated Show resolved Hide resolved
lib/libconfigcpp.c++ Outdated Show resolved Hide resolved
@muesli4
Improve exception error messages
2519815 
* Inherit exceptions from std::runtime_error
* Refactor error message functions
* Refactor path construction functions
* Add path to error messages
@muesli4
Copy link
Author

muesli4 commented Dec 29, 2022

It has been a long time but I took I few minutes trying to get this done. So, I guess better late than never. Let me know if you want anything changed (even small details).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyperrealm hyperrealm hyperrealm left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@muesli4 @ilyapopov @hyperrealm