[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

wallace abandoned this revision. wallace added a comment. not needed anymore because of D99974 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80659/new/ https://reviews.llvm.org/D80659

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

labath added a comment. Sorry, I confused this with the patch that was wrapping stderr into DAP messages. Having python output break the protocol is definitely not good. Fixing it would be nice, but the solution should work on windows as well. One possibility I'd consider is making this fix

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

aadsm added a comment. > Why would we be doing something (particularly a thing which will be hard to > do in a cross-platform manner, and will very likely border on, or downright > cross into, undefined behavior territory), if we get that from vscode for > free? I'm not sure what you mean

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

labath added a comment. Sorry, I missed this patch. Modulo the inline comments and the missing windows support and tests, I think this patch is ok-ish. However, I am still not convinced of its usefulness. Why would we be doing something (particularly a thing which will be hard to do in a

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

aadsm updated this revision to Diff 276894. aadsm added a comment. This update still doesn't add windows support but I just want to get a feel that I'm going on the right direction. It addresses the following: - redirect stdout/err as early as possible to avoid something writing to it, and

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

rmaz added a comment. In D80659#2062304 , @labath wrote: > Btw, given that Richard has found some vscode code which already tries to > send stderr to the console window >

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

aadsm added a comment. Absolutely, I'll look into that first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80659/new/ https://reviews.llvm.org/D80659 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

labath added a comment. Btw, given that Richard has found some vscode code which already tries to send stderr to the console window https://github.com/microsoft/vscode/blob/master/src/vs/workbench/contrib/debug/node/debugAdapter.ts#L234, it would be good to first understand why doesn't that

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

aadsm added a comment. thanks for the feedback, I'll read it more carefully later. As for the tests it should be easy to set up, didn't want to spend time on that before we agreed on doing it this way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

labath added a comment. I don't actually use vscode, so I don't really know whether this is a good idea, but if you say it is then I can go with that. However, I think the implementation needs a lot of work: - it is very windows-hostile: `dup`, `pipe`, `select`, etc. don't exist or don't work

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

aadsm created this revision. aadsm added reviewers: clayborg, labath, rmaz. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Doing this for 2 main reasons: - Avoid stdout from corrupting DAP messages - Make sure we can see in VSCode (or any other DAP client) the same output