utils: Make fs::path::string() always return utf-8 string on Windows #13877

pull ken2812221 wants to merge 1 commits into bitcoin:master from ken2812221:fs-path-utf8 changing 4 files +6 −6
  1. ken2812221 commented at 4:43 pm on August 4, 2018: contributor
    Imbue fs::path with std::codecvt_utf8_utf16 at SetupEnvironment(), so that default string encoding will be utf-8 inside fs::path.
  2. ken2812221 force-pushed on Aug 4, 2018
  3. ken2812221 renamed this:
    utils: Make fs::path::string() always return utf-8 string
    utils: Make fs::path::string() always return utf-8 string on Windows
    on Aug 4, 2018
  4. DrahtBot commented at 8:13 pm on August 4, 2018: member
    • #14123 (gui: Add GUIUtil::bringToFront by promag)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  5. ken2812221 force-pushed on Aug 4, 2018
  6. ken2812221 renamed this:
    utils: Make fs::path::string() always return utf-8 string on Windows
    wip, utils: Make fs::path::string() always return utf-8 string on Windows
    on Aug 5, 2018
  7. ken2812221 renamed this:
    wip, utils: Make fs::path::string() always return utf-8 string on Windows
    utils: Make fs::path::string() always return utf-8 string on Windows
    on Aug 5, 2018
  8. ken2812221 force-pushed on Aug 5, 2018
  9. ken2812221 force-pushed on Aug 5, 2018
  10. ken2812221 force-pushed on Aug 5, 2018
  11. ken2812221 force-pushed on Aug 5, 2018
  12. ken2812221 force-pushed on Aug 5, 2018
  13. laanwj added the label Windows on Aug 6, 2018
  14. laanwj added the label Utils/log/libs on Aug 6, 2018
  15. ken2812221 force-pushed on Aug 24, 2018
  16. in src/util.cpp:61 in 26d6fe4a10 outdated 
    57@@ -58,6 +58,7 @@
58 #ifndef NOMINMAX
59 #define NOMINMAX
60 #endif
61+#include <codecvt>
    NicolasDorier commented at 2:10 am on August 31, 2018:
    maybe include only if WIN32?
    ken2812221 commented at 2:44 pm on August 31, 2018:
    OK. But I’ll wait for gitian build done.
    ken2812221 commented at 7:02 am on September 6, 2018:
    This is already be defined only if WIN32. Forgot about this
    MarcoFalke commented at 6:38 pm on September 22, 2018:

    Previously applied patch detected.

    Needs rebase?

    ken2812221 commented at 7:45 pm on September 22, 2018:
    Rebased
  17. MarcoFalke added the label Needs gitian build on Aug 31, 2018
  18. DrahtBot removed the label Needs gitian build on Sep 2, 2018
  19. NicolasDorier commented at 5:49 am on September 6, 2018: contributor
    Done quick code review (on other commits as well), seems ok, I will test more deeply later this week. Very excited about this.
  20. in src/qt/guiutil.cpp:63 in 26d6fe4a10 outdated 
    61@@ -62,8 +62,6 @@
62 #include <QFontDatabase>
63 #endif
64 
65-static fs::detail::utf8_codecvt_facet utf8;
    ken2812221 commented at 7:06 am on September 6, 2018:
    Drop this because this convert between UTF-8 and UCS-2, not UTF-16. Use std::codecvt_utf8_utf16 instead.
    ryanofsky commented at 10:46 pm on September 6, 2018:

    Drop this because this convert between UTF-8 and UCS-2, not UTF-16. Use std::codecvt_utf8_utf16 instead.

    This is documented at https://www.boost.org/doc/libs/1_68_0/boost/detail/utf8_codecvt_facet.hpp, in case anybody else is curious.

  21. ryanofsky approved
  22. ryanofsky commented at 10:47 pm on September 6, 2018: member
    utACK 26d6fe4a10c2c8cd16303835dd7cd5289f2d3b67
  23. in src/qt/guiutil.cpp:765 in 26d6fe4a10 outdated 
    778@@ -781,12 +779,12 @@ void setClipboard(const QString& str)
779 
780 fs::path qstringToBoostPath(const QString &path)
781 {
782-    return fs::path(path.toStdString(), utf8);
783+    return fs::path(path.toStdString());
    laanwj commented at 9:51 am on September 13, 2018:
    doesn’t this break support for non-UTF8 path locales on UNIX? (that’s why this code was how it was, AFAIK)
    ken2812221 commented at 10:00 am on September 13, 2018:
    that utf8 is unused on other platform than Windows https://github.com/boostorg/filesystem/blob/16821f7903f02d0bb6bb2ef2503ea41559c6bdf1/include/boost/filesystem/path.hpp#L411-455
    laanwj commented at 12:29 pm on September 13, 2018:
    strange… (I don’t think it was added for windows back in the day, but I might be misremembering)
    ken2812221 commented at 12:36 pm on September 13, 2018:
    @laanwj You only mention about Windows in #3935
  24. MarcoFalke deleted a comment on Sep 14, 2018
  25. ryanofsky commented at 6:32 pm on September 21, 2018: member

    Done quick code review (on other commits as well), seems ok, I will test more deeply later this week. Very excited about this. @NicolasDorier, are you still planning on testing this? If not, I think it would be good to get this merged since #13878 depends on it.

    Note to reviewers: even though fix here is pretty esoteric, it should be hopefully should be clear that it doesn’t effect anything other than fs::path encodings used on windows.

  26. MarcoFalke added this to the milestone 0.18.0 on Sep 21, 2018
  27. MarcoFalke added the label Needs gitian build on Sep 21, 2018
  28. DrahtBot commented at 1:13 pm on September 22, 2018: member

    Gitian builds for commit 920c090f63f4990bf0f3b3d1a6d3d8a8bcd14ba0 (master):

    Gitian builds for commit af0794a4f1e0a892780150c6069154f8327cb1e2 (master and this pull):

  29. DrahtBot removed the label Needs gitian build on Sep 22, 2018
  30. Make fs::path::string() always return utf-8 string 2c3eade704
  31. ken2812221 force-pushed on Sep 22, 2018
  32. MarcoFalke commented at 8:30 pm on September 22, 2018: member
    utACK 2c3eade704f63b360926de9e975ce80143781679 (Only checked that this does’t affect linux)
  33. laanwj commented at 11:33 am on September 23, 2018: member
    utACK 2c3eade704f63b360926de9e975ce80143781679
  34. MarcoFalke merged this on Sep 25, 2018
  35. MarcoFalke closed this on Sep 25, 2018
  36. MarcoFalke referenced this in commit cc7258bdfb on Sep 25, 2018
  37. ken2812221 deleted the branch on Sep 25, 2018
  38. Warrows referenced this in commit fbec2557ef on Oct 14, 2019
  39. Warrows referenced this in commit d88bdc5aaa on Nov 23, 2019
  40. Munkybooty referenced this in commit 21b3d1556d on Jul 10, 2021
  41. Munkybooty referenced this in commit f6d9b44801 on Jul 10, 2021
  42. Munkybooty referenced this in commit 7f1c2c0d89 on Jul 11, 2021
  43. Munkybooty referenced this in commit aa5cb13a18 on Jul 12, 2021
  44. Munkybooty referenced this in commit 7de0b07a92 on Jul 12, 2021
  45. Munkybooty referenced this in commit 4749c7e566 on Jul 13, 2021
  46. Munkybooty referenced this in commit e8c2117b16 on Jul 13, 2021
  47. UdjinM6 referenced this in commit 6aea99094b on Jul 13, 2021
  48. random-zebra referenced this in commit 61a098a775 on Aug 5, 2021
  49. MarcoFalke locked this on Sep 8, 2021


ken2812221 DrahtBot NicolasDorier MarcoFalke ryanofsky laanwj

Labels
Windows Utils/log/libs

Milestone
0.18.0

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: 2024-07-01 13:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me