scottaxcell / winddcutil

Windows implementation of the ddcutil Linux program for querying and changing monitor settings, such as brightness and color levels.
https://github.com/scottaxcell/winddcutil
MIT License
168 stars 21 forks source link

Return 0 on success in getvcp/setvcp #3

Closed jtai closed 1 year ago

jtai commented 1 year ago

The getVcp and setVcp methods return the boolean results of the underlying API calls, which causes the program to exit 1 on success and 0 on failure. Typically 0 indicates success, so here's a small change to return the appropriate exit codes.

Due to the encoding of the .cpp file, git seems to think it's binary, so unfortunately the diff doesn't appear correctly. I've reproduced the diff below for easier review:

diff --git a/winddcutil/winddcutil.cpp b/winddcutil/winddcutil.cpp
index 6042067..9bc33ec 100644
--- a/winddcutil/winddcutil.cpp
+++ b/winddcutil/winddcutil.cpp
@@ -121,14 +121,14 @@ int getVcp(std::vector<std::string> args) {
        bool success = GetVCPFeatureAndVCPFeatureReply(physicalMonitorHandle, vcpCode, NULL, &currentValue, NULL);
        if (!success) {
                std::cerr << "Failed to get the vcp code value" << std::endl;
-               return success;
+               return 1;^M
        }

        std::stringstream ss;
        ss << std::hex << currentValue;
        std::cout << "VCP " << args[1] << " " << ss.str() << std::endl;

-       return success;
+       return 0;^M
 }

 int setVcp(std::vector<std::string> args) {
@@ -156,9 +156,11 @@ int setVcp(std::vector<std::string> args) {
        }

        bool success = SetVCPFeature(physicalMonitors[displayId].hPhysicalMonitor, vcpCode, newValue);
-       if (!success)
+       if (!success) {^M
                std::cerr << "Failed to set vcp feature" << std::endl;
-       return success;
+               return 1;^M
+       }^M
+       return 0;^M
 }

 std::unordered_map<std::string, std::function<int(std::vector<std::string>)>> commands
scottaxcell commented 1 year ago

Looks good, thanks for the improvement!

jtai commented 1 year ago

I rebased against your latest commit 961d857ece2faba1b59ba3faa412f0f45ae51fe0 to resolve the merge conflict.