EXEC command removes comments in strings

[sonyps5201314]

function print_args(args)
    print(args)
end

use below code:

local _, result, err = mobdebug.handle(string.format("exec return print_args('%s')", '1/0 --[[math.huge]]'), client)

result is:

but execute from local like below code:

print_args('1/0 --[[math.huge]]')

result is:

[pkulchenko]

@sonyps5201314, the following patch should fix the issue:

diff --git a/src/mobdebug.lua b/src/mobdebug.lua
index dc4ea68..6dcdb6b 100644
--- a/src/mobdebug.lua
+++ b/src/mobdebug.lua
@@ -1381,9 +1381,7 @@ local function handle(params, client, options)
     local _, _, exp = string.find(params, "^[a-z]+%s+(.+)$")
     if exp or (command == "reload") then
       if command == "eval" or command == "exec" then
-        exp = (exp:gsub("%-%-%[(=*)%[.-%]%1%]", "") -- remove comments
-                  :gsub("%-%-.-\n", " ") -- remove line comments
-                  :gsub("\n", " ")) -- convert new lines
+        exp = exp:gsub("\n", "\r") -- convert new lines, so the fragment can be passed as one line
         if command == "eval" then exp = "return " .. exp end
         client:send("EXEC " .. exp .. "\n")
       elseif command == "reload" then

The comment-removing logic was too primitive to handle comments inside strings.

[sonyps5201314]

No, it will cause more problem, and it still can not solve '\n' char was replaced to '\r' in string variable parameter.
In my program, I use below code to send a function to remote client:

local mobdebug = require "mobdebug"
local function GetFunctionCodeText(func)
    local info = debug.getinfo(func, "S")
    local filename = info.source
    if filename:sub(1, 1) == "@" then
        filename = filename:sub(2)
    end
    local func_text
    local file = io.open(filename, "rb")
    assert(file)
    if file then
        local lines = file:lines()
        local cur_line = 0
        for line in lines do
            cur_line = cur_line + 1
            if cur_line > info.lastlinedefined then
                break
            elseif cur_line == info.linedefined then
                func_text = line
            elseif cur_line > info.linedefined then
                func_text = func_text .. "\n" .. line
            end
        end
        io.close(file)
    end
    return func_text
end

function CheckMobDebugVersionIsMatched(server_version)
    local mobdebug_client = require "mobdebug"
    return mobdebug_client._VERSION == server_version
end

local function SendMyRemoteDebugCodes(client, funcs_text)
    local _, err
    if funcs_text:len() > 0 then
        _, _, err = mobdebug.handle('exec ' .. funcs_text, client)
    end
    assert(err == nil)
    return err == nil
end
SendMyRemoteDebugCodes(client, GetFunctionCodeText(CheckMobDebugVersionIsMatched))

before use your patch file , the function work ok at least, but after use the patch file , it will assert error at below line:

  assert(err == nil)

so I think the patch file is still not fix the problem, and cause new problem, I fix this problen in this issue by add a new command "execdirect" like the follow patch, use the new command will fix the problem in this issue.

 src/mobdebug.lua | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mobdebug.lua b/src/mobdebug.lua
index dc4ea68..4e81863 100644
--- a/src/mobdebug.lua
+++ b/src/mobdebug.lua
@@ -1376,6 +1376,7 @@ local function handle(params, client, options)
       end
     end
   elseif command == "eval" or command == "exec"
+      or command == "execdirect"
       or command == "load" or command == "loadstring"
       or command == "reload" then
     local _, _, exp = string.find(params, "^[a-z]+%s+(.+)$")
@@ -1386,6 +1387,8 @@ local function handle(params, client, options)
                   :gsub("\n", " ")) -- convert new lines
         if command == "eval" then exp = "return " .. exp end
         client:send("EXEC " .. exp .. "\n")
+      elseif command == "execdirect" then
+        client:send("EXEC " .. exp .. "\n")
       elseif command == "reload" then
         client:send("LOAD 0 -\n")
       elseif command == "loadstring" then

but i think it is a compromise solution, maybe you have a perfect solution, thanks.

[pkulchenko]

No, it will cause more problem, and it still can not solve '\n' char was replaced to '\r' in string variable parameter.

@sonyps5201314, I don't think it will cause more problems, as the current method replaces it with a space and removes comments; the suggested patch will only replace it with \r, which in most cases will behave similar to \n.

I do agree that adding another command may address this, but it won't change the behavior of the EXEC command; since it may need to be updated, it will make it incompatible with the current behavior. Adding execdirect the way you suggested won't work if the fragment that is being evaluated includes \n, which will break the protocol interactions.

I started with a patch that only removes comments outside of strings, but this still doesn't solve the problem of replacing \n with spaces, so I like my proposed replacement (\n to \r) much better.

[sonyps5201314]

for example，if use exec send a text file to client and get checksum，because \n replaced with \r， so they will be different checksum result.
and you can try my code mentioned above，if no use your patch file， it can run work，but use the patch file will have a runtime error.
and if use the new execdirect command，that is user`s responsibility to stringify the text to one line string，can use mobdebug.line to complete this task.
and you can rename the new 'execdirect' command to like 'execonelinedstring' to help user to understand its fucntion.

[pkulchenko]

if use exec send a text file to client and get checksum，because \n replaced with \r， so they will be different checksum result.

I do realize that there may be cases where the processing is going to be affected. All I'm saying is that the fix I proposed is still better than the existing logic (even though implementing an additional command may cover all cases).

Having a new command still doesn't solve the problem with the behavior of the existing command, so I'd rather fix it as I proposed and wait for more feedback.

[sonyps5201314]

@pkulchenko,
I am the first feedback user of the new version of MobDebug.lua you want to release soon.
You can use the following patch file to easily reproduce the problem I mentioned. It will not let ZeroBraneStudio report an error before using the patch you provided. After using the patch, ZeroBraneStudio will report an error.

 src/editor/debugger.lua | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/src/editor/debugger.lua b/src/editor/debugger.lua
index 526ddd86..dffeeb5a 100644
--- a/src/editor/debugger.lua
+++ b/src/editor/debugger.lua
@@ -52,6 +52,47 @@ end
 local q = EscapeMagic
 local MORE = "{...}"
 
+local function GetFunctionCodeText(func)
+    local info = debug.getinfo(func, "S")
+    local filename = info.source
+    if filename:sub(1, 1) == "@" then
+        filename = filename:sub(2)
+    end
+    local func_text
+    local file = io.open(filename, "rb")
+    assert(file)
+    if file then
+        local lines = file:lines()
+        local cur_line = 0
+        for line in lines do
+            cur_line = cur_line + 1
+            if cur_line > info.lastlinedefined then
+                break
+            elseif cur_line == info.linedefined then
+                func_text = line
+            elseif cur_line > info.linedefined then
+                func_text = func_text .. "\n" .. line
+            end
+        end
+        io.close(file)
+    end
+    return func_text
+end
+
+function CheckMobDebugVersionIsMatched(server_version)
+    local mobdebug_client = require "mobdebug"
+    return mobdebug_client._VERSION == server_version
+end
+
+local function SendMyRemoteDebugCodes(client, funcs_text)
+    local _, err
+    if funcs_text:len() > 0 then
+        _, _, err = mobdebug.handle('exec ' .. funcs_text, client)
+    end
+    assert(err == nil)
+    return err == nil
+end
+
 function debugger:init(init)
   local o = {}
   -- merge known self and init values
@@ -681,6 +722,9 @@ function debugger:Listen(start)
       -- set basedir first, before loading to make sure that the path is correct
       debugger:handle("basedir " .. debugger.basedir)
 
+      local client = debugger.server
+      local funcs_text = GetFunctionCodeText(CheckMobDebugVersionIsMatched)
+      SendMyRemoteDebugCodes(client, funcs_text)
       local init = options.init or ide.config.debugger.init
       if init then
         local _, _, err = debugger:execute(init)

use your patch file before:

use your patch file after: