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

autocomplete entity names in the repl #798

Merged
merged 2 commits into from Oct 29, 2022
Merged

autocomplete entity names in the repl #798

merged 2 commits into from Oct 29, 2022

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Oct 28, 2022

No description provided.

kwMatches = filter (lastWord `T.isPrefixOf`) possibleWords

-- checks the "parity" of the number of quotes. If odd, then there is an open quote.
hasOpenQuotes = (== 1) . (`mod` 2) . T.count "\""
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it's worth fixing this, but technically this will break if there are any escaped quote characters in a string literal.

Copy link
Member

Choose a reason for hiding this comment

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

This approach sounds good enough for now. 👍

Realistically, I think complex programs using escaped quotes will be written in the editor, not REPL.
If we add suggestions to the LSP, then we can think about a more robust check.

theEntityMap = s ^. gameState . entityMap
entityNames = M.keys $ entitiesByName theEntityMap
lastQuotedString = T.takeWhileEnd (/= '"') t
strMatches = filter (lastQuotedString `T.isPrefixOf`) entityNames
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this could be nicer if we filtered using some Map function and then got the list of keys.

But maybe that is just useless optimization. 🤷

Comment on lines 794 to 795
[m] -> CmdPrompt (completeWith lastQuotedString m) []
(m : ms) -> CmdPrompt (completeWith lastQuotedString m) (ms ++ [m])
Copy link
Member

Choose a reason for hiding this comment

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

I forgot what the idea was with this completing code.

I would expect it to cycle on the suggestions, which makes me wonder why the "one element" and "more element" branches can not be the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added explanatory comments.

@kostmo kostmo marked this pull request as ready for review October 29, 2022 06:35
@kostmo
Copy link
Member Author

kostmo commented Oct 29, 2022

Code is now cleaned up. Will merge if I get an "Approve".

@kostmo kostmo self-assigned this Oct 29, 2022
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

This looks great, and thanks for adding the detailed comments!

src/Swarm/TUI/Controller.hs Outdated Show resolved Hide resolved
@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Oct 29, 2022
@mergify mergify bot merged commit 8c01fc1 into main Oct 29, 2022
@mergify mergify bot deleted the autocomplete-strings branch October 29, 2022 17:13
kostmo added a commit that referenced this pull request Dec 10, 2022
mergify bot pushed a commit that referenced this pull request Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants