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

csv overrides #63

Merged
merged 55 commits into from
Aug 31, 2021
Merged

csv overrides #63

merged 55 commits into from
Aug 31, 2021

Conversation

AndreasArvidsson
Copy link
Member

@AndreasArvidsson AndreasArvidsson commented Aug 18, 2021

To do:

  • Add docs
  • Link to docs from before every defaults list
  • Make "cell" a proper scope type
  • Support user removing an item either by commenting it or using special identifier
  • Surrounding pair types
  • Compound range terms
  • Minor cheatsheet cleanup
  • Change "list specifier" to "list connective"?
  • Make csv location customizable
  • Use range specifier for subtoken ranges
  • Change "box" to "square", then change back a week or two after release so new users will have that default
  • Move range connective stuff to connective.py and use it for bring / move / swap
  • Use user-defined connectives in cheatsheet and switch to f-strings
  • Update docs to mention configurable location
  • Update doc string for csv watcher func
  • Figure out exclude / include for subtokens
  • See if we need a mapping for scope type deprecations
  • Make "inside" / "outside" available everywhere again

Won't do:

  • Positions (treatment of these is still in flux, so maybe let's hold off for now)
  • Make "string" into a surrounding pair instead of a syntactic scope

@AndreasArvidsson AndreasArvidsson marked this pull request as draft August 18, 2021 11:16
@pokey
Copy link
Member

pokey commented Aug 18, 2021

Fwiw I get the following error when checking out this branch:

2021-08-18 12:34:00 ERROR user.cursorless-talon.src.cheat_sheet (/Users/pokey/.talon/user/cursorless-talon/src/cheat_sheet.py) import failed
   15:               lib/python3.9/threading.py:912* # cron thread
   14:               lib/python3.9/threading.py:954* 
   13:               lib/python3.9/threading.py:892* 
   12:                            talon/cron.py:155| 
   11:                            talon/cron.py:106| 
   10:                              talon/fs.py:64 | 
    9:                              talon/fs.py:57 | 
    8:                  talon/scripting/rctx.py:233| # 'fs' main:on_change()
    7:                  app/resources/loader.py:689| 
    6:                  app/resources/loader.py:639| 
    5:                  app/resources/loader.py:413| # [stack splice]
    4:      lib/python3.9/importlib/__init__.py:127| 
    3:                  app/resources/loader.py:243| 
    2:                  app/resources/loader.py:238| 
    1: user/cursorless-talon/src/cheat_sheet.py:6  | from .actions.actions import action_list_names
ImportError: cannot import name 'action_list_names' from 'user.cursorless-talon.src.actions.actions' (/Users/pokey/.talon/user/cursorless-talon/src/actions/actions.py)

But everything works great, and when I reload talon I don't get the error on startup. Seems fine to me

@AndreasArvidsson
Copy link
Member Author

There probably she reason why relative imports is discouraged

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

Looks great with a bunch of minor comments. Also feel free to merge #64 into your branch if it looks good. As I was hacking, I noticed that cursorless was pretty much bricked if there was an error on one line of a csv, which is kind of against the talon philosophy of "yell, but plough on if you can"

Also, a couple other thoughts:

  • It might be worth having a special spoken form value that users can use to disable an action / scope type they don't want. Eg <DISABLED>, setSelection
  • What happens if we ever remove an action / scope type? I guess we prob shouldn't do that as it breaks backwards compatibility, but I could see us merging scope types / actions or something

src/actions/actions.py Show resolved Hide resolved
src/actions/actions.py Outdated Show resolved Hide resolved
src/actions/actions.py Outdated Show resolved Hide resolved
src/actions/actions.py Outdated Show resolved Hide resolved
src/actions/actions.py Show resolved Hide resolved
src/modifiers/containing_scope.py Show resolved Hide resolved
src/modifiers/containing_scope.py Show resolved Hide resolved
src/modifiers/selection_type.py Outdated Show resolved Hide resolved
src/cursorless.talon Show resolved Hide resolved
src/csv_overrides.py Outdated Show resolved Hide resolved
AndreasArvidsson and others added 4 commits August 18, 2021 17:52
Co-authored-by: Pokey Rule <pokey.rule@gmail.com>
Co-authored-by: Pokey Rule <pokey.rule@gmail.com>
Co-authored-by: Pokey Rule <pokey.rule@gmail.com>
Co-authored-by: Pokey Rule <pokey.rule@gmail.com>
@AndreasArvidsson
Copy link
Member Author

I don't think we need a voice command to disable a command. Is better to just edit the override file.
-, setSelection or something similar.

Removing an identifier would be handled the same way as renaming. ie with an update list

@pokey
Copy link
Member

pokey commented Aug 18, 2021

I don't think we need a voice command to disable a command. Is better to just edit the override file.
-, setSelection or something similar.

exactly, but I think <DISABLED>, setSelection, as per my original message, would be clearer than a dash

Removing an identifier would be handled the same way as renaming. ie with an update list

yep makes sense. let's try to avoid it but agreed that's prob the right way to handle it if / when it happens

Copy link
Member Author

@AndreasArvidsson AndreasArvidsson left a comment

Choose a reason for hiding this comment

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

Looks good
Do we want to handle inside outside before merging?

This was linked to issues Aug 31, 2021
Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

wait one sec. going to disable shapes for this release to give us a couple days to tweak

@pokey pokey merged commit 6ce114b into develop Aug 31, 2021
@pokey pokey deleted the overrides branch August 31, 2021 16:00
This was referenced Aug 31, 2021
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.

Review all identifiers Add action term override csv
2 participants