Skip to content
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

Completion caching (simplified) #148

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
35fc573
Improved prompt regexp, can match at bol
jdtsmith May 24, 2019
be932f2
lua-shell-temp-file: temp file for sending long commands to lua
jdtsmith May 24, 2019
0591197
Custom variables for completion query
jdtsmith May 24, 2019
6b126e9
lua-send-command-output-to-buffer-and-wait: new send modality
jdtsmith May 24, 2019
9dfbb12
lua-start-of-expression: written
jdtsmith May 24, 2019
1d9f69e
lua process querying caching completion framework
jdtsmith May 24, 2019
424b130
lua-start-process: process buffer-locality and send redirection
jdtsmith May 24, 2019
6aadffa
Clean up after comint-redirect
jdtsmith May 24, 2019
f2cd92d
Makefile: Switch from sed to perl for better portability
jdtsmith May 24, 2019
fa1013e
test-completion: testing framework added
jdtsmith May 24, 2019
54d1528
README: describe new completion capability
jdtsmith May 24, 2019
316d0c2
Drop support for Emacs 24.3 and add 26.2
jdtsmith May 24, 2019
c038e78
Increment version string
jdtsmith May 24, 2019
eb29c18
emacs_lua_complete: Add spaces for clarity
jdtsmith May 25, 2019
0c59a28
Setup scripts/ directory for lua scripts
jdtsmith May 25, 2019
c618207
emacs_lua_complete: Add spaces for clarity [TOSQUASH]
jdtsmith May 25, 2019
9085997
Setup scripts/ directory for lua scripts [TOSQUASH]
jdtsmith May 25, 2019
a678055
Be explicit about function symbol [TOSQUASH]
jdtsmith May 25, 2019
b713a31
Merge branch 'completion-caching-simplified' of github.com:jdtsmith/l…
jdtsmith May 25, 2019
1d928e1
Remove superfluous comint-redirect [TOSQUASH]
jdtsmith Jun 10, 2019
e307d61
maximum-command-length -> maximum-sent-length [TOSQUASH]
jdtsmith Jun 10, 2019
863e9df
Removed unused lua-shell-last-command [TOSQUASH]
jdtsmith Jun 10, 2019
623642d
Don't use REGEXP search for parts match, and allow _ starting [TOSQUASH]
jdtsmith Jun 12, 2019
5697ea8
prefer ipairs for looping over parts [TOSQUASH]
jdtsmith Jun 14, 2019
0045b51
Document limit better, and whitespace
jdtsmith Jun 14, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ addons:
packages:
- lua5.2
env:
- EVM_EMACS=emacs-24.3-travis
- EVM_EMACS=emacs-24.4-travis
- EVM_EMACS=emacs-24.5-travis
- EVM_EMACS=emacs-25.1-travis
- EVM_EMACS=emacs-25.2-travis
- EVM_EMACS=emacs-25.3-travis
- EVM_EMACS=emacs-26.2-travis
# - EVM_EMACS=emacs-git-snapshot-travis
before_install:
- curl -fsSkL https://gist.github.com/rejeep/ebcd57c3af83b049833b/raw > travis.sh && source ./travis.sh
- evm install "$EVM_EMACS" --use --skip
install:
- cask install
script:
- make
- make test
8 changes: 6 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Makefile for lua-mode

VERSION="$(shell sed -nre '/^;; Version:/ { s/^;; Version:[ \t]+//; p }' lua-mode.el)"
VERSION="$(shell perl -ne 'if(s/^;; Version:[ \t]+(.*)/$$1/){print; exit}' lua-mode.el)"
DISTFILE = lua-mode-$(VERSION).zip

# EMACS value may be overridden
Expand All @@ -10,8 +10,12 @@ LUA_MODE_ELC=lua-mode.$(EMACS_MAJOR_VERSION).elc

EMACS_BATCH=$(EMACS) --batch -Q

mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST)))
mkfile_dir := $(dir $(mkfile_path))
export LUA_PATH = $(mkfile_dir)test/?.lua

default:
@echo version is $(VERSION)
@echo version is $(VERSION), LUA_PATH is $(LUA_PATH)

%.$(EMACS_MAJOR_VERSION).elc: %.elc
mv $< $@
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ hash-bang line (`#!/usr/bin/lua`). Putting this snippet to `.emacs` should be en
- documentation lookup (using online/offline reference manual, e.g. [string.find](http://www.lua.org/manual/5.1/manual.html#pdf-string.find))
- [imenu](http://www.gnu.org/software/emacs/manual/html_node/emacs/Imenu.html) integration
- [HideShow](http://www.gnu.org/software/emacs/manual/html_node/emacs/Hideshow.html) integration
- using `completion-at-point` queries active Lua subprocess for completion targets

## CUSTOMIZATION

Expand All @@ -56,6 +57,7 @@ The following variables are available for customization (see more via `M-x custo
- Var `lua-mode-hook`: list of functions to execute when lua-mode is initialized
- Var `lua-documentation-url` (default `"http://www.lua.org/manual/5.1/manual.html#pdf-"`): base URL for documentation lookup
- Var `lua-documentation-function` (default `browse-url`): function used to show documentation (`eww` is a viable alternative for Emacs 25)
- Var `lua-local-require-completions` (default `t`): set to `nil` to prevent completion attempting to load local libraries for current file for completion targets

## LUA SUBPROCESS CREATION

Expand Down
81 changes: 81 additions & 0 deletions emacs_lua_complete.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
-- __emacs_lua_complete:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice step to move this away, it is clearly way too complicated to include in lua-mode.el file verbatim. One extra thing to do here is to figure out the packaging part. For example, we could put this into scripts/ directory, and update MELPA recipe and the Cask file to include (:files :defaults ("scripts", "*")).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. It also saves constant recompilation every time you complete, and opens the door for more "behind-the-scenes" interaction with lua going forward.

The scripts sounds like a good plan. I created a scripts directory and referenced it. I'm not really familiar with Cask; I think you have push access to this branch, if you don't mind sending a commit over.

-- This function finds and prints a limited number of matches on the
-- lua globals tree, _G. See `lua-complete-string`. N.B.: Does not
-- complete numeric keys (e.g. for arrays) or keys which begin with an
-- underscore.
-- Arguments:
-- PARTS: a table array of dot-separated parts of the completion item
-- (e.g. table.con would become {"table","con"}). All but the last
-- element must be valid subkeys in the _G heirarchy.
-- LIBS: an array of tables of the form
-- {var: varname, lib: libfile}
-- for e.g. local varname=require("libfile"). This library will be
-- require'd and searched alongside global variables.
-- LOCALS: an array of strings parsed from local aloc,bloc = ... to
-- complete as well
--
-- Example:
-- __emacs_lua_complete({'table','ins'},{{var="xyz",lib="libfile"}},
-- {"frank","beans"},5000)
--
-- Written 2019/05 J.D. Smith
function __emacs_lua_complete(parts,libs,locals,limit)
-- Setup a FIFO queue for breadth-first tree traversal
local queue={head=0,tail=0}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming from Python I feel like this line is missing whitespaces around equals signs (or binary operators). What are the common style guidelines in Lua world? I saw some packages defining whitespace requirements, and some just deferring to "whatever is the default in PIL".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lua is very lenient about whitespace in either direction, but I can certainly add it in for visual clarity. Added. BTW, I will alter history in this branch after the fact to avoid the clutter of small commits like these, so please don't rely on specific commit ID's!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's please add whitespace around binary operators and newlines where necessary, now that there's no necessity to keep the size to the minimum.

And things like

    local globals,cnt,limit={},0,limit or 2000 

could probably use a touch to clarify (limit assignment in particular catches me off guard every time I return to this piece of code).

function queue:push(value)
self.tail=self.tail+1 self[self.tail]=value
end
function queue:pop()
self.head=self.head+1 return self[self.head]
end

-- Mirror the globals table
local globals,cnt,limit={},0,limit or 2000
for k,v in pairs(_G) do globals[k] = v end

-- Add libs and locals
if libs then
for _,v in ipairs(libs) do
local good,lib=pcall(require,v.lib)
globals[v.var]=good and lib or {}
end
end
if locals then
for _,v in ipairs(locals) do globals[v]={} end
end

-- Descend the tree to the limit of dotted parts
local g,pre=globals,""
for i=1,#parts do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would ipairs work here to replace all the parts[i] in the body of the loop?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I use i to tell me if I'm in the "last" part, which may be a partial match (or even an empty string for table.[M-Tab] type completions). But I'm by no means a lua expert, so let me know if there's a better lua way.

Copy link
Owner

@immerrr immerrr Jun 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, what I'm saying is that you are basically doing

for i = 1, #parts do
    current_part = parts[i]

whereas ipairs will do that for you automatically

for i, current_part in ipairs(parts)

and you get to keep both i and current_part in the loop's body.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, fixed.

if not g then break end -- should always exist
if i<#parts then -- keep descending
pre = (i > 1 and pre.."." or "")..parts[i]
g = g[parts[i]]
else -- final part; seed the queue with any matching tables
for k,v in pairs(g) do
if type(k)=="string" and k:sub(1,1) ~= "_" and k:find('^'..parts[i]) then
local dpath=(i > 1 and pre.."." or "")..k
print(dpath)
immerrr marked this conversation as resolved.
Show resolved Hide resolved
cnt=cnt+1
if type(v)=="table" then queue:push({pre=dpath,entries=v}) end
end
end
end
end

-- Perform a limited breadth-first traversal of the queued tables
while queue.head < queue.tail and cnt < limit do
local q=queue:pop()
for k,v in pairs(q.entries) do
if type(k) == "string" and k:sub(1,1) ~= "_" then
pre = q.pre.."."..k
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember you told me that when keys come from completions, they are verified that they don't contain whitespaces or dots? Here, if I'm not mistaken, k comes from Lua tables and no verification is done, and thus k can contain whatever and produce completions that are invalid syntax or don't work as expected. WDYT?

Copy link
Author

@jdtsmith jdtsmith Jun 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Yeah any valid key in Lua can be in k, including keys with whitespace. But you can't reference such keys with dot-syntax (I just tried). Since we are only parsing and completing dot-syntax I think we can get away with it. It would be better of course to parse a[" foo "].b[" c"] style things too, but that's a new can of worms. If that ever comes to pass, you'd trim and recreate whitespace around dots, but NOT within brackets (since there it is significant and must be preserved). I guess it might not be that hard to take an arbitrary expression with brackets and dots (and comments) and turn it into a series of parts with all the ignorable whitespace removed and all the relevant whitespace retained. Right now it's this simple bit:

 (mapcar 'lua-completion-trim-input (split-string expr "\\."))

Wouldn't be terrible to update this to take the expression and remove comments, find brackets etc. Right now lua-start-of-expression doesn't handle brackets gracefully.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the simplest solution is to just filter out any keys that are not alphanumeric-plus-underscore here.

print(pre)
cnt = cnt + 1
if cnt >= limit then break end
if type(v) == "table" then queue:push({pre=pre,entries=v}) end
end
end
end
end


Loading