[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

MaskRay added inline comments. Comment at: llvm/lib/Support/MemoryBuffer.cpp:460 - if (shouldUseMmap(FD, FileSize, MapSize, Offset, RequiresNullTerminator, -PageSize, IsVolatile)) { + if (false) { std::error_code EC; I guess this was

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

This revision was automatically updated to reflect the committed changes. Closed by commit rG127faae7529a: [lldb] Add -l/--language option to script command (authored by JDevlieghere). Herald added subscribers: llvm-commits, hiraditya. Herald added projects: LLDB, LLVM. Changed prior to commit:

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere added a comment. In D86996#2273445 , @labath wrote: > This looks fine (sorry about the delay, I've been OOO). I hope you had a good time. Glad to have you back! :-) Comment at:

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks fine (sorry about the delay, I've been OOO). Comment at: lldb/test/Shell/ScriptInterpreter/Lua/lua.test:2 # REQUIRES: lua -# RUN: %lldb --script-language lua -o

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere updated this revision to Diff 290596. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86996/new/ https://reviews.llvm.org/D86996 Files: lldb/source/Commands/CommandObjectScript.cpp lldb/source/Commands/CommandObjectScript.h lldb/source/Commands/Options.td

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Sounds reasonable to me. I'll split it up in two patches. I'll update this one to use the double dashes so we can land the functionality and keep things consistent with the rest of LLDB. I'll create another patch to do

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

labath added a comment. I'm not married to the current way we process commands, but I do value their consistency (both between different commands, and between a command and their documentation). This would make `script` behave differently than `expr` even though they have identical modes of

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

teemperor added a comment. I have to agree with @kastiglione that this isn't very intuitive. I doubt that more than a handful of users are aware that "raw commands" are a thing, so I think this inconsistency is for once in favor of the user. LLDB commands are anyway not meant to be stable, so

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

kastiglione added a comment. > Given the `expr` status quo, I'd stick with that. I want to give a +1 to //not// copying `expr`. I think the tradeoff is not not worth it. My user perspective is that lldb could be improved by making more decisions with the user in mind, not lldb's engineers (to

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. In D86996#2252561 , @JDevlieghere wrote: > In D86996#2252520 , @JDevlieghere > wrote: > >> In

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Commands/CommandObjectScript.cpp:37 +"default", +"The default scripting language.", +}, kastiglione wrote: > is the default's name

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

kastiglione added inline comments. Comment at: lldb/source/Commands/CommandObjectScript.cpp:37 +"default", +"The default scripting language.", +}, is the default's name available in a `define`? If maybe it'd be good to include it here? I

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere added a comment. In D86996#2252520 , @JDevlieghere wrote: > In D86996#2252246 , @labath wrote: > >> I agree that silly, but maybe the same fix should then be applied to the >> `expr` command (and any

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86996/new/ https://reviews.llvm.org/D86996 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere added a comment. In D86996#2252246 , @labath wrote: > I agree that silly, but maybe the same fix should then be applied to the > `expr` command (and any other command with similar behavior). Sure, we can use the same trick in other places

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

labath added a comment. In D86996#2251508 , @JDevlieghere wrote: > because I thought it would be silly to require you to type `script --language > lua --` to launch the interactive Lua interpreter. For better or worse, this is how `expr` works:

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere added a comment. In D86996#2251485 , @teemperor wrote: > So CommandObjectRaw does support arguments, but they way it works is that you > need to have a delimiter for the 'raw' part which is `--`. If you have that, > then you can just use

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere updated this revision to Diff 289369. JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. Herald added a subscriber: dang. - Remove custom parsing and use the command options insofar possible. - Require `--` as a delimiter when language and code are specified

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

teemperor requested changes to this revision. teemperor added a comment. So CommandObjectRaw does support arguments, but they way it works is that you need to have a delimiter for the 'raw' part which is `--`. If you have that, then you can just use invoke the normal option parsing like

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere planned changes to this revision. JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Commands/CommandObjectScript.cpp:50-51 + + std::tie(head, tail) = tail.split(' '); + if (head != "-l" && head !=

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

kastiglione added inline comments. Comment at: lldb/source/Commands/CommandObjectScript.cpp:50-51 + + std::tie(head, tail) = tail.split(' '); + if (head != "-l" && head != "--language") +return CommandParsed(command); generally, lldb supports more than

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

kastiglione added inline comments. Comment at: lldb/source/Commands/CommandObjectScript.cpp:65 + + return CommandParsed(command); +} this is missing error handling for unsupported languages, such as `script -l swift` Comment at:

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

JDevlieghere created this revision. JDevlieghere added reviewers: labath, mib, l.frisken, jingham. JDevlieghere requested review of this revision. Make it possible to run the `script` command with a different language than currently selected. $ ./bin/lldb -l python (lldb) script -l lua