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

add default message for pvget #195

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

rerpha
Copy link

@rerpha rerpha commented Apr 16, 2024

Adds a default message for pvget. I don't know if line 363 (the default case) was a bug as i could never get it to run. @ralphlange I found this after I showed you my (accidentally?) working PVAccess.

Maybe there's a good reason that pvget doesn't have this line but the other tools do, please let me know!

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@mdavidsaver
Copy link
Member

I don't know if line 363 (the default case) was a bug as i could never get it to run.

That default case should be impossible so long as the switch is consistent with the string passed to getopt(). imo. any inconsistency should be an error. I usually combine default and '?' though.

Maybe there's a good reason that pvget doesn't have this line but the other tools do, please let me know!

I guess you are referring to caget?

It think it is a bit of a judgement call whether PVs is an error with pvget. With pvmonitor I think this should be.

Is it worth changing behavior at this point?

@rerpha
Copy link
Author

rerpha commented May 20, 2024

Maybe there's a good reason that pvget doesn't have this line but the other tools do, please let me know!

I guess you are referring to caget?

It think it is a bit of a judgement call whether PVs is an error with pvget. With pvmonitor I think this should be.

Is it worth changing behavior at this point?

by other tools yes caget, but also pvput, pvinfo, pvmonitor all seem to give the "No pv name(s) specified. ('pvXXXX -h' for help.)" message - just thinking for consistency among the pv* utils it might be nice.

pvtoolsSrc/pvget.cpp Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

Comment on lines +373 to +374
usage();
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the indentation, these should line up with the fprintf() above.

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

Successfully merging this pull request may close these issues.

4 participants