Skip to content

Attempt to optimize dfa #57

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

Merged
merged 22 commits into from
Aug 15, 2021
Merged

Attempt to optimize dfa #57

merged 22 commits into from
Aug 15, 2021

Conversation

pokey
Copy link
Member

@pokey pokey commented Aug 9, 2021

I kept almost all functionality, but dropped a couple things I never use, and the grammar is maybe a bit more awkward

Curious what people think

One of my takeaways here is that DFA compilation seems to be quicker if big captures appear towards the end of a command rather than the beginning. @lunixbochs does that sound right to you?

  • "replace T with X" should change to "change T word X". We no longer support arbitrary format strings here. We can then add "add" with same syntax "add T word X", but by default will modify T to have an "after". In the future, it will be its own action (see Switch targets to object-oriented cursorless#210)
  • change the recursive descent code for “after”
  • it’s "[inner | outer | pair] {enclosingScopeType} | (inner | outer | pair)"
  • And remove insideOutsideType
  • And switch “tween” to “between”
  • And switch bringSwapMove grammar back

fixes #50

@pokey pokey changed the base branch from master to develop August 9, 2021 21:55
@pokey pokey requested a review from AndreasArvidsson August 9, 2021 21:55


mod.list("cursorless_multiple_target_action", desc="Cursorless move or bring actions")
ctx.lists["self.cursorless_multiple_target_action"] = {"bring", "move", "swap", "call"}
Copy link
Member Author

Choose a reason for hiding this comment

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

Unified the syntax for all these multi-target commands. Slightly awkward for some of them but not too bad so far; will have to see how well it works for a couple days

Copy link
Member

Choose a reason for hiding this comment

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

I total understand why you did it but I'm not quite happy with it :s


replace <user.cursorless_target> with <user.cursorless_replace_value>$:
user.cursorless_replace(cursorless_target, cursorless_replace_value)

reformat <user.cursorless_target> as <user.formatters>:
format <user.formatters> onto <user.cursorless_target>:
Copy link
Member Author

Choose a reason for hiding this comment

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

ok so this one is pretty awkward, but surprisingly swapping these two captures seems to improve DFA compilation. We could prob come up with better terminology here

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should just remove onto and make it format camel blue air

@lunixbochs
Copy link

lunixbochs commented Aug 10, 2021 via email


mod.list("cursorless_pair_symbol", desc="A symbol that comes in pairs, eg brackets")
ctx.lists["self.cursorless_pair_symbol"] = {
"skis": "backtickQuotes",
Copy link
Member

Choose a reason for hiding this comment

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

Can't we make do with just brick wrap?

MakeshiftAction("drink cell", "jupyter.insertCellAbove"),
MakeshiftAction("follow", "editor.action.revealDefinition"),
Copy link
Member

Choose a reason for hiding this comment

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

Personally I use reveal definition much more than I use find. Maybe we should rename find instead?

@pokey pokey added this to the 0.21.0 milestone Aug 14, 2021
@AndreasArvidsson AndreasArvidsson linked an issue Aug 14, 2021 that may be closed by this pull request
@AndreasArvidsson AndreasArvidsson merged commit bbcffe7 into develop Aug 15, 2021
@AndreasArvidsson AndreasArvidsson deleted the pokey-dfa-optimization branch August 15, 2021 11:31
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.

Change “if” to “if state”
3 participants