nealsid added a comment. In D106035#2879939 <https://reviews.llvm.org/D106035#2879939>, @teemperor wrote:
> I actually expected after the RFC that we would remove all the non-wchar > code, but this seems also fine. I think this LGTM in general, but I feel a > bit nervous about touching stuff that depends so much on OS/environment. What > OS/environment did you test this patch on? Would be nice to have this tested > on a few setups before landing. This was my mistake, sorry. I originally went the route in this patch and ran into some errors testing, so I switched to what I detailed in the RFC. But then I found the problem (I was using narrow chars for the GetCharacter callback when that actually isn't supported). Overall I think it is best to use narrow char and string rather than wide-char and wstring because that's more consistent with the rest of LLVM. Regarding platforms, I tested on OS X Big Sur and Monterey, and Debian Linux inside a VM. Jan was able to build on Fedora. I'm happy to test on more platforms - FreeBSD, NetBSD perhaps? > Also I'm kinda curious if you found any docs/examples that explain whether > mixing the wchar/char functions like we do now is actually supported in > libedit? IIUC we call now `el_wset` and `el_set` on the same libedit > instance. It feels like the `wchar` support in the FreeBSD port was some kind > of parallel implementation to the normal `char` support, so I'm surprised > that we can just mix those functions and everything still works fine (at > least on my Linux machine this seems to work). Yeah, the original source converts the parameters and calls el_w* functions when the narrow-char functions are called. This is also true on FreeBSD: https://github.com/freebsd/freebsd-src/blob/373ffc62c158e52cde86a5b934ab4a51307f9f2e/contrib/libedit/eln.c#L359 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106035/new/ https://reviews.llvm.org/D106035 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
