MaxOSX: settings fixes (#2371) #2613

pull jonasschnelli wants to merge 2 commits into bitcoin:master from jonasschnelli:prefsFix changing 4 files +60 −5
  1. jonasschnelli commented at 1:19 PM on May 3, 2013: contributor
    • Launch-At-Startup implementation for mac
    • Remove "Window" tab in settings
  2. jonasschnelli commented at 1:20 PM on May 3, 2013: contributor

    launch at startup can easily be tested while having the mac settings app open (show at your users startup items while checking/unchecking the start-at-login setting).

  3. jonasschnelli commented at 1:22 PM on May 3, 2013: contributor

    Proxy un-setting is still a issue. Will also analyze the proxy-unsetting-bug...

  4. jonasschnelli commented at 1:56 PM on May 3, 2013: contributor

    I would say that SOCKS proxy unsetting works as expected.

    If you unset them, the app warns you that you have to restart Bitcoin-Qt to make use of your new settings. When you restart the app, SOCKS proxys are off as expected. When you not restart the app, SOCKS proxys are still "enabled" in the settings (so you can uncheck them again and again). Placing an additional text ("your need to restart,...") in the settings window looks like an overhead. Showing the proxy in off state (while the app current acts over the proxy) sounds also as a no-go.

  5. BitcoinPullTester commented at 1:58 PM on May 3, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/7c7e314e187a85286250f5b06db39b4906e01f2a for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  6. in src/qt/guiutil.cpp:None in 7c7e314e18 outdated 
     418 | +
 419 | +#include <CoreFoundation/CoreFoundation.h>
 420 | +#include <CoreServices/CoreServices.h>
 421 | +
 422 | +LSSharedFileListItemRef findStartupItemInList(LSSharedFileListRef list, CFURLRef findUrl);
 423 | +LSSharedFileListItemRef findStartupItemInList(LSSharedFileListRef list, CFURLRef findUrl) {
    Diapolo commented at 4:31 PM on May 3, 2013:

    Looks a little weird to have function declaration and definition right below eachother? Also can you please use that style:

    <pre> void foo() { bar(); } </pre>

    jonasschnelli commented at 6:01 PM on May 4, 2013:

    okay. will change it to the bitcoin common code style.

    jonasschnelli commented at 6:00 PM on May 5, 2013:

    The function declaration next to the definition looks good for me. Because it's preprocessor based mac only code it would say it's acceptable.

  7. in src/qt/guiutil.cpp:None in 7c7e314e18 outdated 
     421 | +
 422 | +LSSharedFileListItemRef findStartupItemInList(LSSharedFileListRef list, CFURLRef findUrl);
 423 | +LSSharedFileListItemRef findStartupItemInList(LSSharedFileListRef list, CFURLRef findUrl) {
 424 | +    // loop through the list of startup items and try to find the bitcoin app
 425 | +    CFArrayRef listSnapshot = LSSharedFileListCopySnapshot(list, NULL);
 426 | +    for(int i=0;i<CFArrayGetCount(listSnapshot);i++) {
    Diapolo commented at 4:32 PM on May 3, 2013:

    Nit: Missing some spaces (find it pretty ugly and hard to read that way): for(int i = 0; i < CFArrayGetCount(listSnapshot); i++) {

  8. in src/qt/guiutil.cpp:None in 7c7e314e18 outdated 
     427 | +        LSSharedFileListItemRef item = (LSSharedFileListItemRef)CFArrayGetValueAtIndex(listSnapshot, i);
 428 | +        UInt32 resolutionFlags = kLSSharedFileListNoUserInteraction | kLSSharedFileListDoNotMountVolumes;
 429 | +        CFURLRef currentItemURL = NULL;
 430 | +        LSSharedFileListItemResolve(item, resolutionFlags, &currentItemURL, NULL);
 431 | +        if(currentItemURL != NULL) {
 432 | +            if(currentItemURL && CFEqual(currentItemURL, findUrl)) {
    Diapolo commented at 4:33 PM on May 3, 2013:

    That currentItemURL check seems redundant, because if it is NOT NULL, it will be set here?

    jonasschnelli commented at 6:04 PM on May 4, 2013:

    your right. will be changed.

  9. in src/qt/guiutil.cpp:None in 7c7e314e18 outdated 
     433 | +                // found
 434 | +                CFRelease(currentItemURL);
 435 | +                return item;
 436 | +            }
 437 | +        }
 438 | +        if(currentItemURL) {
    Diapolo commented at 4:34 PM on May 3, 2013:

    This could be just an else also?

  10. in src/qt/guiutil.cpp:None in 7c7e314e18 outdated 
     444 | +
 445 | +bool GetStartOnSystemStartup() {
 446 | +    CFURLRef bitcoinAppUrl = CFBundleCopyBundleURL(CFBundleGetMainBundle());
 447 | +    LSSharedFileListRef loginItems = LSSharedFileListCreate(NULL, kLSSharedFileListSessionLoginItems, NULL);
 448 | +    LSSharedFileListItemRef foundItem = findStartupItemInList(loginItems, bitcoinAppUrl);
 449 | +    return !!foundItem; // return boolified object
    Diapolo commented at 4:35 PM on May 3, 2013:

    What does !! here, I'm asking because I really don't know :).

    fanquake commented at 4:48 PM on May 3, 2013:

    It turns the return value into either 1 or 0

  11. in src/qt/optionsdialog.cpp:None in 7c7e314e18 outdated 
      45 | @@ -46,6 +46,14 @@
  46 |      /* Window elements init */
  47 |  #ifdef Q_OS_MAC
  48 |      ui->tabWindow->setVisible(false);
    Diapolo commented at 4:45 PM on May 3, 2013:

    IMHO remove this (as setVisible doesn't even work for me, when changing to Q_OS_WIN) and just replace with: ui->tabWidget->removeTab(ui->tabWidget->indexOf(ui->tabWindow));

    So no need for that loop below :).

    jonasschnelli commented at 6:04 PM on May 4, 2013:

    nice! will do that.

    laanwj commented at 6:53 PM on May 7, 2013:

    It is strange that simply hiding doesn't work. But setting it to invisible before removing it seems overkill :)

    jonasschnelli commented at 7:12 PM on May 7, 2013:

    setVisible is on tabWindow not no tabWidget. I just kept this code line...

    Diapolo commented at 7:47 PM on May 7, 2013:

    AFAIK it doesn't work so it would be fine to remove that line.

    jonasschnelli commented at 7:49 PM on May 7, 2013:

    okay. let me try and see.. :)

    Diapolo commented at 7:37 PM on May 19, 2013:

    @jonasschnelli I still vote for removing that unneeded line.

  12. BitcoinPullTester commented at 6:13 PM on May 5, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/11032aa284e76ae155e27c47f2cc7d01d0a33c33 for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in contrib/test-scripts)
    2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
    3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    4. The test suite fails on either Linux i386 or Win32
    5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

    If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

    This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  13. Diapolo commented at 6:58 AM on May 6, 2013: none

    Looks good now, but I obviously can't test the code ;). Btw. pulltester failure is unrelated to the pull: test/util_tests.cpp(308): error in "util_seed_insecure_rand": check count>=10000/mod-err failed

  14. laanwj commented at 7:34 PM on May 19, 2013: member

    ACK after 0.8.2 release

  15. jonasschnelli commented at 11:04 AM on May 24, 2013: contributor

    fixed last minor issue (unused line ui->tabWindow->setVisible(false);).

    Final ACKs?

  16. in src/qt/optionsdialog.cpp:None in d5ae4cba41 outdated 
      44 | @@ -45,7 +45,8 @@
  45 |  
  46 |      /* Window elements init */
  47 |  #ifdef Q_OS_MAC
  48 | -    ui->tabWindow->setVisible(false);
  49 | +    /* remove window tab on mac */
    Diapolo commented at 12:17 PM on May 24, 2013:

    One nit, which doesn't prevent my ACK, I wrote Window starting uppercase and I would always use Mac over mac :D.

    jonasschnelli commented at 12:18 PM on May 24, 2013:

    Okay. Will fix that soon...

  17. BitcoinPullTester commented at 12:54 PM on May 24, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/d5ae4cba418b1c47e7eb6caefdd9009fa6d3f28c for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  18. MaxOSX: settings fixes (#2371)
    - Launch-At-Startup implementation for mac
- Remove "Window" tab in settings

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>
    f679b2900a
  19. jonasschnelli commented at 10:27 AM on June 3, 2013: contributor

    merge?

  20. BitcoinPullTester commented at 11:41 AM on June 3, 2013: none

    Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/f679b2900a3a9f863f888cfb0b1a5e593628e37b for test log.

    This pull does not merge cleanly onto current master This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  21. Merge branch 'master' of git://github.com/bitcoin/bitcoin into prefsFix
    Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

Conflicts:
	bitcoin-qt.pro
    39fe9de6b2
  22. jonasschnelli commented at 1:19 PM on June 3, 2013: contributor

    rebased with master

  23. BitcoinPullTester commented at 4:06 PM on June 3, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/39fe9de6b2b255969971beca8fa25a33ad2e5750 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  24. laanwj referenced this in commit c83d4d2170 on Jun 3, 2013
  25. laanwj merged this on Jun 3, 2013
  26. laanwj closed this on Jun 3, 2013
  27. DrahtBot locked this on Sep 8, 2021
Contributors
jonasschnelliBitcoinPullTesterDiapolofanquakelaanwj
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-18 21:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me