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.

Sign up for GitHub

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

Jump to bottom

Protect against symbols with unsafe chars #649

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Protect against symbols with unsafe chars #649

wants to merge 2 commits into from

Conversation

RuijieYu
Copy link
Contributor

@RuijieYu RuijieYu commented Apr 4, 2023 •
edited
Loading

Fixes #648. Also fixes #653 by its first commit.

  • lispy.el lispy--symbol-safe-chars-regexp: define safe chars.
    (lispy--read): protect against unsafe chars in a symbol.

@simendsjo: Can you check if this works?

@simendsjo
Copy link

@simendsjo: Can you check if this works?

Almost. (.> foo) no longer escapes, but (.) actually removes all three characters.

@RuijieYu RuijieYu force-pushed the bug-648-clisp-dot-in-symbol branch 2 times, most recently from c41d26a to 4203638 Compare April 5, 2023 04:22
@RuijieYu
Copy link
Contributor Author

RuijieYu commented Apr 5, 2023

This iteration should fix the immediate problem (and I have some commits coming after for cleaning up unnecessary code). Let me know if it works. The culprit was that lispy--bounds-dwim wasn't able to get its bounds, and now it should.

@RuijieYu RuijieYu mentioned this pull request Apr 5, 2023
Open
@simendsjo
Copy link

Let me know if it works.

Not sure if I'm doing anything strange or if it's something Doom related which causes problems, but when testing 4203638, pretty much nothing works. Pressing (, I get (), then d to get (d), then trying to add e at the following where | is the cursor (d|) gives forward-list: Scan error: "Containing expression ends prematurely", 1, 1

@RuijieYu
Copy link
Contributor Author

RuijieYu commented Apr 5, 2023

Got it. Looks like I misunderstood what lispy-right represents.

@RuijieYu RuijieYu mentioned this pull request Apr 6, 2023
Open
@RuijieYu
Fixed test file, and added regression test cases
8175e9a 
Fixes bug#653.

* lispy-test.el
(lispy-simulate-key) (lispy-simulate-keys) (lispy-simulate-expect):
Added facilities to assert results after a given set of keypresses.
(lispy-read-unsafe-chars): added regression tests for bug#648.
@RuijieYu
Protect against symbols with unsafe chars
311cfd0 
Fixes bug#648.

* lispy.el lispy--symbol-safe-chars: define list of safe chars.
(lispy--read): protect against unsafe chars in a symbol.
(lispy--replace-regexp-in-code): add predicate to only replace on match data.
@RuijieYu RuijieYu force-pushed the bug-648-clisp-dot-in-symbol branch from 4203638 to 311cfd0 Compare April 7, 2023 08:41
@RuijieYu
Copy link
Contributor Author

RuijieYu commented Apr 7, 2023

Does the current iteration work? I wrote some tests incorporating what you have tested so far, and it seems that the tests have passed on my end.

@simendsjo
Copy link

Does the current iteration work? I wrote some tests incorporating what you have tested so far, and it seems that the tests have passed on my end.

Yes, looks like it's working! Thanks!

@RuijieYu RuijieYu marked this pull request as draft April 30, 2023 03:48
@RuijieYu
Copy link
Contributor Author

Blocked by #658. Don’t have much time ATM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

make test fails Common Lisp formatting changes the code
2 participants
@RuijieYu @simendsjo