-
Notifications
You must be signed in to change notification settings - Fork 2
Support tarantool #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
За исключением пары замечаний про хардкод констант (которые не блокирующие, и можно вернуться к этому позже) всё остальное LGTM.
У нас есть возможность отлаживать встроенные модули в Тарантуле. Woohoo!
5a575d3
to
5f5cd84
Compare
792dc4a
to
bee7a0e
Compare
bee7a0e
to
242ed9f
Compare
15b053f
to
7b2fc46
Compare
- Conditional call to tarantool.debug.getsources() (to make it work with unpatched tarantool executables also);
Conditional tarantool.debug.getsources() usage
2c4505f
to
81e4dc1
Compare
81e4dc1
to
df2d03c
Compare
README.md
Outdated
|
||
 | ||
### Tarantool Lua Stand-Alone Interpreter | ||
To debug a Lua program using a stand-alone interpreter, set `lua-tarantool.interpreter` in your user or workspace settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I assume we now should refer to
lua-tarantool.tarantool
parameter here; - I'd rephrase it slightly (t omake it more Tarantool specific), e.g:
"To debug a Lua program using particular Tarantool executable, set lua-tarantool.tarantool
in your user or workspace settings."
```lua | ||
require("lldebugger").start() | ||
``` | ||
Note that the path to `lldebugger` will automatically be appended to the `LUA_PATH` environment variable, so it can be found by Lua. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd keep this information elsewhere (at the end of standard configuration part?):
"Note that the path to lldebugger
will automatically be appended to the LUA_PATH
environment variable, so it can be found by Lua."
@@ -213,7 +213,7 @@ export class LuaDebugSession extends LoggingDebugSession { | |||
env: Object.assign({}, process.env), | |||
cwd, | |||
shell: true, | |||
detached: process.platform !== "win32" | |||
detached: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also cleanup usage of detached
as it's always true
now.
: ""; | ||
const processArgs: string[] = [ | ||
"-e", | ||
"\"require 'strict'.off()\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Soon after our discussion I've realized that we do not need to mess with strict
mode at all (even under debugging), as we do not use any global variable anymore (as it used to be before), but rather simple require "tarantool".debug
which is perfectly compatible with strict
mode.
i.e. those 2 lines adding call to "require 'strict'.off()" are not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we do need to have it enabled as there is rawlen
used
package.json
Outdated
@@ -20,9 +20,6 @@ | |||
"tstl", | |||
"typescripttolua", | |||
"typescript-to-lua", | |||
"love", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there (with a few documentation changes, and single, small code tweek)
No description provided.