[Lldb-commits] [PATCH] D49963: Preliminary patch to support prompt interpolation

nealsid accepted this revision. nealsid added inline comments. This revision is now accepted and ready to land. Comment at: include/lldb/Host/Editline.h:187 + /// Register a callback to retrieve the prompt. + void SetPromptCallback(PromptCallbackType callback, void *baton); +

[Lldb-commits] [PATCH] D49963: Preliminary patch to support prompt interpolation

nealsid added a comment. In https://reviews.llvm.org/D49963#1183036, @clayborg wrote: > This will be very cool. Biggest issues to watch out for is to make sure it > doesn't return incorrect values when running for things like the thread > count. If you switch to use the > "m_debugger.GetComman

[Lldb-commits] [PATCH] D50299: Migrate to std::function instead of static member functions for callbacks

nealsid updated this revision to Diff 163548. nealsid added a comment. Fix 80 char line length violations Repository: rLLDB LLDB https://reviews.llvm.org/D50299 Files: include/lldb/Core/IOHandler.h include/lldb/Host/Editline.h source/Core/IOHandler.cpp source/Host/common/Editline.cpp

[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

nealsid updated this revision to Diff 163559. nealsid added a comment. Ran clang-format Repository: rLLDB LLDB https://reviews.llvm.org/D50299 Files: include/lldb/Core/IOHandler.h include/lldb/Host/Editline.h source/Core/IOHandler.cpp source/Host/common/Editline.cpp unittests/Editl

[Lldb-commits] [PATCH] D115126: [LLDB} Add unit tests for Editline keyboard shortcuts

nealsid created this revision. nealsid added a reviewer: LLDB. nealsid added a project: LLDB. Herald added a subscriber: JDevlieghere. nealsid requested review of this revision. Herald added a subscriber: lldb-commits. This patch adds tests that verify keyboard shortcuts are set up for multi-line

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid updated this revision to Diff 391976. nealsid added a comment. Fix casing typo in friend class declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115126/new/ https://reviews.llvm.org/D115126 Files: lldb/include/lldb/Host/Editline

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid added a comment. In D115126#3173236 , @labath wrote: > All of the changes seem fine, but it's not clear to me what is the value of > the new test. To me, it just seems like a change-detector forcing the two > versions of the keybindings to stay

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

nealsid added a comment. In D113650#3168622 , @clayborg wrote: > I can't build on macOS now. I checked out the source code and tried to do: > > cmake -G Ninja -DCMAKE_BUILD_TYPE:STRING=Debug > -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE > -DLLVM_ENABLE_PROJECT

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid added a comment. Random unrelated question, and if I missed docs somewhere, I apologize. Did I forget something in creating the revision that caused Harbormaster to not do arc lint or arc test? https://reviews.llvm.org/B137600. Thanks. Repository: rG LLVM Github Monorepo CHANGES S

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid updated this revision to Diff 392497. nealsid edited the summary of this revision. nealsid added a comment. Testing ARC, no action necessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115126/new/ https://reviews.llvm.org/D115126 Files:

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid added a comment. Thank you, Pavel - will address the specific code feedback in subsequent patch. In D115126#3178906 , @labath wrote: > Well.. I would say that the most user-facing aspect is the /action/ that > happens after the user pressed the

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid updated this revision to Diff 393200. nealsid added a comment. Patch to address CR feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115126/new/ https://reviews.llvm.org/D115126 Files: lldb/include/lldb/Host/Editline.h lldb/sourc

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid added a comment. Thanks, Pavel. Comment at: lldb/unittests/Editline/EditlineTest.cpp:133-139 + // We have to set the output stream we pass to Editline as not using + // buffered I/O. Otherwise we are missing editline's output when we + // close the stream in the key

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid updated this revision to Diff 393228. nealsid added a comment. Remove extra header and using decl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115126/new/ https://reviews.llvm.org/D115126 Files: lldb/include/lldb/Host/Editline.h lldb/

[Lldb-commits] [PATCH] D115474: [lldb] Refactor & update of EditlineTest

nealsid created this revision. Herald added a subscriber: kristof.beyls. nealsid requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This patch has a few minor cleanups and a restructuring of the EditlineTest class: Cleanups: - Adds some comm

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

nealsid added a comment. Thanks, Pavel. Some comments responded to, will take another look at the buffered/unbuffered I/O issue and address the remaining comments. Comment at: lldb/include/lldb/Host/Editline.h:392-393 + + friend class EditlineKeyboardBindingTest_MultiLineEdi

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

nealsid created this revision. nealsid added a reviewer: LLDB. nealsid requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This small patch adds a test for functions & lldb commands with unicode characters in them. Repository: rG LLVM Gith

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

nealsid updated this revision to Diff 352612. nealsid added a comment. Herald added a subscriber: JDevlieghere. Add newline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104437/new/ https://reviews.llvm.org/D104437 Files: lldb/test/Shell/Breakp

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

nealsid added a comment. Ah, my grep/find skills clearly failed me :) Yeah, those tests are exactly the same scenarios. However, if I understand correctly, don't they use the API? I wanted to add some coverage for the shell because I'm making changes to the Editline wrapper, and the existing t

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

nealsid added a comment. Thanks to both of you for this! I’m traveling for a few days but will be able to take a look towards the end of the week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104437/new/ https://reviews.llvm.org/D104437

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

nealsid added a comment. Agreed, it looks like there's plenty of test coverage for this scenario, so I'll revert this patch - sorry about that. I'm working on removing the wchar/char preprocessor logic in Editline.cpp so I'll verify the existing coverage before sending that for review. Thanks

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

nealsid abandoned this revision. nealsid added a comment. Not needed with existing test coverage. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104437/new/ https://reviews.llvm.org/D104437 ___ lldb-commi

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid created this revision. nealsid added a reviewer: LLDB. nealsid added a project: LLDB. Herald added subscribers: JDevlieghere, mgorny. nealsid requested review of this revision. Herald added a subscriber: lldb-commits. This change moves to using narrow character types and libedit APIs, beca

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid added a comment. In 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 th

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid updated this revision to Diff 359533. nealsid added a comment. Reran clang-format and removed extra braces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106035/new/ https://reviews.llvm.org/D106035 Files: lldb/cmake/modules/LLDBConfig.c

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid added a comment. In D106035#2885292 , @teemperor wrote: > In D106035#2885174 , @nealsid wrote: > >> In D106035#2879939 , @teemperor >> wrote: >> >>> I actually ex

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid updated this revision to Diff 363358. nealsid added a comment. Update against HEAD (I still need to do a bit more testing but wanted to get the buildbot results in the meantime) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106035/new/ htt

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid updated this revision to Diff 364362. nealsid added a comment. Update against HEAD; tested on Debian Buster and Monterey Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106035/new/ https://reviews.llvm.org/D106035 Files: lldb/cmake/modules

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid added a comment. In D106035#2927660 , @teemperor wrote: > The release branch was recently made, so we can land this right now. Just > give me a ping when I should merge this :) Thanks - I recently got commit access so I figured I'd try it out wi

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid added a comment. :) Shoot. I definitely want to revert ASAP rather than keep the tree in a bad state. Just curious, since there are timeout messages in the log, would you be able to try rerunning just that one? I see this, too: 0: b"error: '\xe1\x88\xb4' is not a valid command. So I'm

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid added a comment. Yikes - issue count starting to slowly increase :) I've reverted the patch, and sorry again about that. I have to travel for a couple days but when I get my dev machine back up later this week I will investigate these issues in detail. I focussed my testing more on Un

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

nealsid added a comment. Sorry, I haven't forgotten about this. Haven't had time to sit down for a proper debugging session this or last week. I definitely still plan to update and resend for review. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

nealsid updated this revision to Diff 326553. nealsid added a comment. Herald added a project: LLDB. I'm updating this very old patch that I forgot about in 2018. This one removes the use of batons in the edit line callbacks, replaces typedefs with using declarations, and uses unique_function i

[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

nealsid added a comment. In D50299#2589289 , @JDevlieghere wrote: > Thanks Neal. Can you run clang-format again and upload the diff with context > (`git diff -U`). Overall this looks pretty good. Thanks for the review. I have run clang-format and a

[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

nealsid updated this revision to Diff 326954. nealsid removed a project: LLDB. nealsid added a comment. Updated diff with run of clang-format and added a check for function pointer validity. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50299/new/ https://reviews.llvm.org/D502

[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

nealsid updated this revision to Diff 327317. nealsid added a comment. Removed comment on #include line and organized the #includes to match coding guidelines better (I wasn't sure about whether the project uses blank lines in between #includes from different subproject like lldb/clang/llvm so I

[Lldb-commits] [PATCH] D50299: Migrate to llvm::unique_function instead of static member functions for callbacks

nealsid added a comment. In D50299#2596141 , @JDevlieghere wrote: > In D50299#2596008 , @nealsid wrote: > >> Removed comment on #include line and organized the #includes to match coding >> guidelines better (I was

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 328917. nealsid added a comment. Herald added a subscriber: JDevlieghere. Reuploading patch after I incorrectly set project repo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid added a comment. Thanks - all the comments sound good. I'll upload a new patch. It seems like the pre-merge check failures are because I incorrectly set the project repo when I first created the revision. I noticed in the build commands here: https://buildkite.com/llvm-project/premer

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 329266. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/FormatEntity.cpp lldb/unittests/Core/CMakeList

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked 3 inline comments as done. nealsid added inline comments. Comment at: lldb/include/lldb/Core/FormatEntity.h:144 +num_children(num_children), children(children), +keep_separator(keep_separator) {} }; teemperor wrote: > I

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked 2 inline comments as done. nealsid added a comment. In D98153#2610953 , @teemperor wrote: > Thanks for cleaning this up! Are the `if (!sc) ...` stuff are missing nullptr > checks that affect a normal LLDB session (not sure if we can ever ha

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid planned changes to this revision. nealsid added inline comments. Comment at: lldb/source/Core/FormatEntity.cpp:1560 } -return true; } Sorry, just caught this. Am uploading a new diff shortly. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 329834. nealsid added a comment. Fix regression introduced by removing return statement in case where we were able to format a function name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://re

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 330884. nealsid added a comment. Rebased on HEAD Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/Forma

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid added a comment. In D98153#2610953 , @teemperor wrote: > Thanks for cleaning this up! Are the `if (!sc) ...` stuff are missing nullptr > checks that affect a normal LLDB session (not sure if we can ever have no > SymbolContext) or is this just fo

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 333507. nealsid added a comment. Thanks for the review - addressed your comments here (I had to rely on Doxygen picking up the type reference automatically because I couldn't get the \see syntax to work correctly, though) Repository: rG LLVM Github Monor

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 337329. nealsid added a comment. CR feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/FormatEnt

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 337331. nealsid added a comment. Fixed comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Core/Format

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 337334. nealsid marked an inline comment as done. nealsid added a comment. Update type in for loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/includ

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked an inline comment as done. nealsid added inline comments. Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154 +TEST(FormatEntity, LookupAllEntriesInTree) { + for (const auto &testString : lookupStrings) { +Entry e; teemperor wrote: > teem

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid marked an inline comment as done. nealsid added a comment. Thanks for the review, and apologies for the delay in updating the diffs - seems like I haven't even had an hour to sit down at my dev machine for the past couple weeks, much less actually do any coding :) I don't have commit ac

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

nealsid updated this revision to Diff 339059. nealsid added a comment. Small update of patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98153/new/ https://reviews.llvm.org/D98153 Files: lldb/include/lldb/Core/FormatEntity.h lldb/source/Cor

[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

nealsid created this revision. nealsid requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Currently we call el_set directly to configure the editor in the libedit wrapper. There are some cases in which this causes extra casting, but we pass

[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

nealsid added a comment. In D101250#2718107 , @JDevlieghere wrote: > LGTM. I wonder if we would want to wrap this in a macro to get rid of the > `EditLineConstString` duplication while keeping the type safety. Thanks! I looked into removing the EditLi

[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

nealsid added a comment. Friendly ping - let me know if I misunderstood the comments or if anything else is needed! Thanks again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101250/new/ https://reviews.llvm.org/D101250 _

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

nealsid created this revision. nealsid added reviewers: LLDB, kparzysz. Herald added subscribers: hiraditya, krytarowski, mgorny. nealsid requested review of this revision. Herald added projects: LLDB, LLVM. Herald added subscribers: llvm-commits, lldb-commits. I don't mean to undo others' work bu

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

nealsid updated this revision to Diff 344489. nealsid added a comment. Updated the diff after moving an unrelated change into another revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102208/new/ https://reviews.llvm.org/D102208 Files: lld

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

nealsid added a comment. I ran LLDB tests from a clean tree and with my change and the failures seemed to be the same, but there is a large amount of them. I also did manual testing with the changes. Build is still green. Editline is only used when Cmake can find the editline libraries, which

[Lldb-commits] [PATCH] D102208: Remove Windows editline from LLDB

nealsid added a comment. Thanks, all. If there are no more comments, could someone land it for me? I don't have commit access. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102208/new/ https://reviews.llvm.org/D102208 ___