Skip to content

Fix sending of long code strings to REPL using temporary files (#89) #122

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

arbv
Copy link

@arbv arbv commented Jun 22, 2016

Now we will try to handle long chunk strings (> 500 characters) via temporary files.

@jtecca
Copy link

jtecca commented Mar 31, 2018

Is there anything that can be done (besides fixing the merge conflicts) to get this PR merged? I am running into issues with long buffers and would like to see this merged into the main repo.

I am willing to help if anything is needed.

@immerrr
Copy link
Owner

immerrr commented Apr 2, 2018

Hi, please accept my apologies, this PR somehow flew under the radar for me.

I think this feature has to be off by default, because sending via tempfiles won't work over network. Alternatively, it could be on by default for macOS, and off by default otherwise.

Also, could the Lua part be simplified a bit?

  • only protect the part up to parsing the code by pcall, return a callable chunk from that, and then remove the file itself, and then report the error if it failed or execute the chunk if it didn't
  • use assert on io.open to do the same with less code
  • same for f:read although I don't think it returns an error message (does it?)

@arbv
Copy link
Author

arbv commented Apr 5, 2018

I believe this feature should be on by default on Windows too. I made this PR initially because I had problems on Windows.

@immerrr
Copy link
Owner

immerrr commented Apr 5, 2018

Oh, then I was wrongly under impression that it was about macOS! Then yes, it should be on by default for Windows.

@vacech
Copy link

vacech commented Aug 11, 2018

I just wanted to leave a short note based on my experience with Aquamacs (though I think it's not specific to Aquamacs) and the temporary file code (as posted in 9eddee) which I think needs a fix.

The name of every temporary file written by lua-send-string is added to the recentf-list which soon makes it cluttered with .tmp names and renders the corresponding Open Recent menu unusable. For my usage I modified the code to use the same file repeatedly for the whole session. The other way around is to prevent the temporary file names completely from going into the recent file menu by adding a matching regexp to recentf-exclude (".*/luamode.*\.tmp" should do).

If you think this is an issue that's worth fixing (I personally do think so) leave a comment here. The fix I use is so simple (using a global var to store the name) that I doubt it's worth forking but I can do a fork if you think it's of any use.

@vacech
Copy link

vacech commented Aug 16, 2018

You could also consider simplifying the part of lua-send-string that sends the command to process the temp file to

(process-send-string
     lua-process
     (format "local tmp = '%s'; local _, e = pcall(function () dofile(tmp) end);
 if e then error(e) end; os.remove(tmp)\n" tmp-file))

which for me seems to work well and inserts way less text into the lua REPL.

@aabacchus
Copy link

RE: what platforms is this relevant to, the default LUA_MAXINPUT is 512 for all platforms: https://www.lua.org/source/5.4/lua.c.html#LUA_MAXINPUT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants