porting the .csv files to .talon-list files #1268

saidelike · 2023-09-30T16:15:26Z

The goal of this PR is to:

remove code to generate the .csv files
include the default .talon-list files
port code in .python files to use the new .talon-list instead of the .csv

This initial commit is about porting {user.letter}: alphabet.csv -> letter.talon-list

It includes a .tools/convert_all_csv_files_to_talon-list.py file in a hidden directory so it is not picked up by talon. Running this script gives something like:

C:\Users\User\AppData\Roaming\talon\user\community\.tools> python convert_all_csv_files_to_talon-list.py
Base directory: C:\Users\User\AppData\Roaming\talon\user\community
Skipping unsuppported convertion yet: apps\emacs\emacs_commands.csv
Skipping unsuppported convertion yet: apps\git\git_arguments.csv
Skipping unsuppported convertion yet: apps\git\git_commands.csv
Skipping unsuppported convertion yet: core\app_switcher\app_name_overrides.linux.csv
Skipping unsuppported convertion yet: core\app_switcher\app_name_overrides.mac.csv
Skipping unsuppported convertion yet: core\app_switcher\app_name_overrides.windows.csv
Skipping unsuppported convertion yet: core\homophones\homophones.csv
Skipping unsuppported convertion yet: settings\abbreviations.csv
Skipping unsuppported convertion yet: settings\additional_words.csv
Converting csv file: settings\alphabet.csv -> talon-list file: core\keys\letter.talon-list
Skipping non default csv file settings\bla.csv
Skipping unsuppported convertion yet: settings\file_extensions.csv
Skipping unsuppported convertion yet: settings\search_engines.csv
Skipping unsuppported convertion yet: settings\system_paths.csv
Skipping unsuppported convertion yet: settings\unix_utilities.csv
Skipping unsuppported convertion yet: settings\websites.csv
Skipping unsuppported convertion yet: settings\words_to_replace.csv
Skipping unsuppported convertion yet: tags\emoji\emoji.csv
Skipping unsuppported convertion yet: tags\emoji\emoticon.csv
Skipping unsuppported convertion yet: tags\emoji\kaomoji.csv

This script retrieves all the .csv files from the community/ folder and try to convert them to .talon-list files. It only support converting those that are default on knausj using a whitelist. Atm, only alphabet.csv is implemented and the others are skipped on purpose. Also, a non default bla.csv was created in settings in my local folder in order to show it is skipped.

I am aware that some other PR exist (#1239 and #1240) but the idea of this new PR is to distinguish talon-list changes to make them more likely to be merged sooner than later. And also provide a script that people can run in order to upgrade their own csv if they have modified them.

Later, this script could support porting non default csv files but the users will still need to update their code accordingly to support the new talon-list files so we can't automate it.

I am mainly interested in feedbacks before we port the other .csv files to .talon-list files.

for more information, see https://pre-commit.ci

nriley · 2023-10-14T15:57:57Z

Some thoughts from the grooming session today:

This requires the user have a non-Talon version of python installed. For many less technical users this will not be the case. It might make sense to turn this into a Talon action the way core Talon did when we migrated action definitions out of .talon files.
For users who have customized their non-settings CSV files, we need to have a reasonable migration path. One possible option might be:
1. Convert all of the standard settings CSV files to .talon-list files.
2. Add a comment to the beginning of every standard non-settings CSV file to encourage use of the action to convert your CSV customizations (and delete the corresponding CSV after conversion?)
3. Display a warning (imgui?) if customized non-settings CSV files remain.

- move logic to migration_helpers.py - Auto-convert known and supported CSVs in on_ready - initial example conversion of git_commands

for more information, see https://pre-commit.ci

knausj85 · 2024-01-22T04:03:00Z

I took a swing at making progress on this.

Proposal:

Autoconvert supported CSVs in OnReady + commit default .talon-lists. When the custom *.csv is processed, the existing logic overwrites the .talon-list in-place.
Certain CSVs need to remain as CSVs as the format is either not a list or (2) it's utilized by create_spoken_forms: homophones, emacs_commands, app_name_overrides, abbreviations, file_extensions. I propose we use the new resource.watch functionality for these and keep them as-is.

If this seems reasonable to folks, this is what needs attention:

Need logic to correctly handle files like emoticon.csv and kaomoji.csv
Need to sort out what to do with system_paths. It's probably most robust to make this a host-specific conversion,
Could probably use some unit tests 😓
Need to update edit_text_file.py
Implement action for custom, non-standard CSV conversion?

for more information, see https://pre-commit.ci

This provides a quick and dirty solution for emojis. There's probably a better way to do this.

for more information, see https://pre-commit.ci

- newpath can now be a function - add support for custom header function convert system path as host specific for now.

for more information, see https://pre-commit.ci

…into pr/1268

for more information, see https://pre-commit.ci

tags/emoji/emoji.talon-list

tags/emoji/emoticon.talon-list

tags/emoji/kaomoji.talon-list

add is_first_line_header parameter to the config for each file

for more information, see https://pre-commit.ci

knausj85 · 2024-01-27T23:30:53Z

I'm pretty satisfied with this approach. Reopening for review.

I don't have time to really think through all the edge cases though, so there might be a handful of conversion issues. Perhaps some other folks can help out if motivated.

Could probably use some testing if folks have time.

Great work on the initial implementation @saidelike, btw!

pokey

Looks good! I'd be happy to give this a spin before we hit the final merge button to make sure it works for me; I have a heavily custom fork so should be a good test case

Speaking of test cases, those would be nice to have, but seems difficult

core/keys/letter.talon-list

core/system_paths.py

migration_helpers/migration_helpers.py

Co-authored-by: Pokey Rule <[email protected]>

core/keys/letter.talon-list

- switch to using data class for supported files - added missing conversions

for more information, see https://pre-commit.ci

knausj85 · 2024-08-11T21:37:43Z

What is this going to mean for the subset of users who "do not want to modify community"? The defaults for these files and any modifications will be tracked in Git by default, unlike the settings folder.

Ultimately, talon has evolved and the repo needs to evolve alongside. I ran the converter some months ago, and vastly prefer tracking these files in git:

Consistency with .talon files. Gets us close to a point where one could "delete all talon and talon-list files" and build their own grammar
Reduces mystery for newbs vs autogenerated CSVs
When merging, one can focus on largely python-side changes and mostly ignore .talon-list

FWIW there was some discussion of this issue on
#1382 and
#1221 (comment)

TL,DR:
It's a good step forward: (1) provides us a potential mechanism for creating a more terse grammar in the future, and (2) we can't please everyone :). I think we commit and move on.

or pre-commit will do it for us.

for more information, see https://pre-commit.ci

nriley · 2024-08-31T18:35:51Z

Did a bunch of cleanup on the README. Ready for (hopefully) final review!

knausj85

This looks good to me.

ekuiter · 2024-09-03T06:43:49Z

2024-09-03 08:37:49.116 DEBUG [+] /Users/ek/.talon/user/modules/community/migration_helpers/migration_helpers.py
2024-09-03 08:37:49.122 ERROR cb error topic="ready" cb=on_ready
   23:                                   lib/python3.11/threading.py:1002* # cron thread
   22:                                   lib/python3.11/threading.py:1045* 
   21:                                   lib/python3.11/threading.py:982 * 
   20:                                                 talon/cron.py:153 * 
   19:                                       talon/scripting/rctx.py:235 * # 'cron' main:Cron.interval.<locals>.call()
   18:                                                 talon/cron.py:181 * 
   17:                                                   talon/fs.py:62  * 
   16:                                                   talon/fs.py:58  * 
   15:                                       talon/scripting/rctx.py:235 * # 'fs' main:Loader.on_change()
   14:                                       app/resources/loader.py:751 * 
   13:                                       app/resources/loader.py:716 * 
   12:                                       app/resources/loader.py:425 * 
   11:                          lib/python3.11/importlib/__init__.py:126 * 
   10:                                       app/resources/loader.py:235 * 
    9: user/modules/community/migration_helpers/migration_helpers.py:261 | app.register("ready", on_ready)
    8:                                                  talon/app.py:23  | 
    7: ------------------------------------------------------------------# [stack splice]
    6:                                       talon/scripting/rctx.py:235 | # 'ready' user.modules.community.migration_helpers.migration_helpers:on_ready()
    5: user/modules/community/migration_helpers/migration_helpers.py:258 | actions.user.migrate_known_csv_files()
    4:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    3:                                    talon/scripting/actions.py:62  | 
    2:                                    talon/scripting/actions.py:156 | 
    1:                                    talon/scripting/actions.py:152 | 
KeyError: "Action 'user.migrate_known_csv_files' is not declared."

Is this migration of CSV files supposed to run automatically? If yes, it does not work on my machine. For now, I have to revert this pull request in my setup.

nriley · 2024-09-03T10:53:46Z


2024-09-03 08:37:49.116 DEBUG [+] /Users/ek/.talon/user/modules/community/migration_helpers/migration_helpers.py

2024-09-03 08:37:49.122 ERROR cb error topic="ready" cb=on_ready

   23:                                   lib/python3.11/threading.py:1002* # cron thread

   22:                                   lib/python3.11/threading.py:1045* 

   21:                                   lib/python3.11/threading.py:982 * 

   20:                                                 talon/cron.py:153 * 

   19:                                       talon/scripting/rctx.py:235 * # 'cron' main:Cron.interval.<locals>.call()

   18:                                                 talon/cron.py:181 * 

   17:                                                   talon/fs.py:62  * 

   16:                                                   talon/fs.py:58  * 

   15:                                       talon/scripting/rctx.py:235 * # 'fs' main:Loader.on_change()

   14:                                       app/resources/loader.py:751 * 

   13:                                       app/resources/loader.py:716 * 

   12:                                       app/resources/loader.py:425 * 

   11:                          lib/python3.11/importlib/__init__.py:126 * 

   10:                                       app/resources/loader.py:235 * 

    9: user/modules/community/migration_helpers/migration_helpers.py:261 | app.register("ready", on_ready)

    8:                                                  talon/app.py:23  | 

    7: ------------------------------------------------------------------# [stack splice]

    6:                                       talon/scripting/rctx.py:235 | # 'ready' user.modules.community.migration_helpers.migration_helpers:on_ready()

    5: user/modules/community/migration_helpers/migration_helpers.py:258 | actions.user.migrate_known_csv_files()

    4:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    3:                                    talon/scripting/actions.py:62  | 

    2:                                    talon/scripting/actions.py:156 | 

    1:                                    talon/scripting/actions.py:152 | 

KeyError: "Action 'user.migrate_known_csv_files' is not declared."

Is this migration of CSV files supposed to run automatically? If yes, it does not work on my machine. For now, I have to revert this pull request in my setup.

I ran into this too. I think it's a core Talon bug not recognizing the action even though we call it from a ready callback. The conversion should run when Talon is restarted, but not sure if we should try to work around this from community.

jaresty · 2024-09-03T14:43:15Z

I ran into this too and opted to manually migrate my alphabet though I'm not sure what other talon lists I may have missed. I wonder if it would be helpful to have a develop branch of the community repository where people could try out new features like this before they hit the main branch.

nriley · 2024-09-03T16:09:32Z

That’s what the backlog sessions are for. And the PRs. I’m not sure what more involvement there is going to be beyond what we’re getting if we add yet more hoops for the very small number of people who meaningfully contribute to community. Anyway, see the community channel on the slack for more discussion.

jaresty · 2024-09-03T16:41:13Z

All good. For my part, I would gladly use a develop branch and provide feedback on it, which could intercept some issues before they hit main. I can't make it to the backlog sessions (unfortunately) and don't have time to check out each PR individually so any feedback I provide on those would be just based on reading them, not based on my actual experience using them (hence not quite as authoritative, IMO).

Besides being another hoop, a develop branch might free up merging PRs more easily since there would be another opportunity for feedback before merging to main. It also allows for PRs to be integrated and issues that come from integration to be discovered sooner. (that's how I've seen them used, anyhow)

nriley · 2024-09-07T20:02:07Z

Some updates from discussion this week and in the community backlog session this morning:

ready is clearly firing too soon before the action is appropriately registered. According to aegis this is fixed for him, but there has not yet been a beta release incorporating this fix and it may be some time until the fix is in the public version of talon.
Instead of working around the bug we decided to simply display a notification if the bug occurs indicating that you should restart Talon. I filed Display message when encountering core Talon issue with ready callback. #1558 with this change.

…k. (#1558) See discussion starting at #1268 (comment)

knausj85 · 2024-09-07T21:26:14Z

#1558 has been merged FYI

ekuiter · 2024-09-07T21:44:31Z

thanks for investigating and taking care of this!

Use `hostname:` rather than `host:` to (correctly) match a hostname. Generate a default `system_paths-<hostname>.talon-list` akin to the prior CSV. Also makes some minor tweaks to the generated spoken forms: - only use "profile" for Windows; "home" for other platforms - add "talon recordings" (because I want to use it in the wiki)

Cedric Halbronn and others added 2 commits September 30, 2023 17:05

porting {user.letter}: alphabet.csv -> letter.talon-list

9da8a32

[pre-commit.ci] auto fixes from pre-commit.com hooks

fc65644

for more information, see https://pre-commit.ci

knausj85 and others added 3 commits January 21, 2024 21:50

Various enhancements

c6c9851

- move logic to migration_helpers.py - Auto-convert known and supported CSVs in on_ready - initial example conversion of git_commands

Delete convert_all_csv_files_to_talon-list.py

18107f9

[pre-commit.ci] auto fixes from pre-commit.com hooks

c070eb2

for more information, see https://pre-commit.ci

knausj85 marked this pull request as draft January 22, 2024 03:52

knausj85 and others added 11 commits January 21, 2024 22:31

rerun conversion after minor bugfix

34b6c45

[pre-commit.ci] auto fixes from pre-commit.com hooks

2999327

for more information, see https://pre-commit.ci

Add support for custom_value_converter.

11f4de8

This provides a quick and dirty solution for emojis. There's probably a better way to do this.

[pre-commit.ci] auto fixes from pre-commit.com hooks

834b998

for more information, see https://pre-commit.ci

Initial support for migrating system_paths

5f456db

- newpath can now be a function - add support for custom header function convert system path as host specific for now.

[pre-commit.ci] auto fixes from pre-commit.com hooks

19c2142

for more information, see https://pre-commit.ci

updates

0099121

[pre-commit.ci] auto fixes from pre-commit.com hooks

bf39692

for more information, see https://pre-commit.ci

merge more conversions

c428439

Merge branch 'talon-list' of https://github.com/saidelike/knausj_talon …

ff9d9d3

…into pr/1268

[pre-commit.ci] auto fixes from pre-commit.com hooks

3b60fac

for more information, see https://pre-commit.ci

saidelike commented Jan 27, 2024

View reviewed changes

tags/emoji/emoji.talon-list Show resolved Hide resolved

tags/emoji/emoticon.talon-list Outdated Show resolved Hide resolved

tags/emoji/kaomoji.talon-list Show resolved Hide resolved

knausj85 and others added 2 commits January 27, 2024 17:26

Support files without a header

e03cab1

add is_first_line_header parameter to the config for each file

[pre-commit.ci] auto fixes from pre-commit.com hooks

19d430d

for more information, see https://pre-commit.ci

knausj85 marked this pull request as ready for review January 27, 2024 23:28

pokey reviewed Jan 29, 2024

View reviewed changes

Update migration_helpers/migration_helpers.py

bdf17fa

Co-authored-by: Pokey Rule <[email protected]>

pokey reviewed Jan 30, 2024

View reviewed changes

core/keys/letter.talon-list Outdated Show resolved Hide resolved

knausj85 and others added 3 commits February 4, 2024 10:08

tweaks

e5c18be

- switch to using data class for supported files - added missing conversions

[pre-commit.ci] auto fixes from pre-commit.com hooks

14de788

for more information, see https://pre-commit.ci

fix comment

c7a5e92

[pre-commit.ci] auto fixes from pre-commit.com hooks

69c689b

for more information, see https://pre-commit.ci

nriley and others added 8 commits August 31, 2024 10:55

Merge branch 'main' into talon-list

fb607a9

Don't need to use newline="" when writing.

d9b0753

Don't display (expected) messages for no or missing CSV conversion.

119e4ec

Update list description.

803bd02

Terminate converted Talon list files in a newline...

35ab509

or pre-commit will do it for us.

Update README.md to reflect current state of community.

762e17e

[pre-commit.ci] auto fixes from pre-commit.com hooks

d1051e9

for more information, see https://pre-commit.ci

Merge branch 'main' into talon-list

eb9dc7e

nriley marked this pull request as ready for review August 31, 2024 18:35

nriley added 2 commits August 31, 2024 14:39

Small README.md grammar fix/cleanup.

93a01cf

Better update README.md for CSV to Talon list conversion.

02d14e2

nriley added a commit to nriley/TalonWiki that referenced this pull request Aug 31, 2024

Update for when talonhub/community#1268 is merged.

8cc217c

knausj85 approved these changes Sep 1, 2024

View reviewed changes

nriley merged commit 940fe69 into talonhub:main Sep 1, 2024
2 checks passed

nriley mentioned this pull request Sep 7, 2024

Display message when encountering core Talon issue with ready callback. #1558

Merged

knausj85 pushed a commit that referenced this pull request Sep 7, 2024

Display message when encountering core Talon issue with ready callbac…

2894c36

…k. (#1558) See discussion starting at #1268 (comment)

nriley added a commit that referenced this pull request Sep 7, 2024

Fix regressions in system path support from #1268. (#1559)

d426076

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

porting the .csv files to .talon-list files #1268

porting the .csv files to .talon-list files #1268

saidelike commented Sep 30, 2023

nriley commented Oct 14, 2023 •

edited

Loading

knausj85 commented Jan 22, 2024 •

edited

Loading

knausj85 commented Jan 27, 2024

pokey left a comment

knausj85 commented Aug 11, 2024

nriley commented Aug 31, 2024 •

edited

Loading

knausj85 left a comment

ekuiter commented Sep 3, 2024

nriley commented Sep 3, 2024

jaresty commented Sep 3, 2024

nriley commented Sep 3, 2024 via email •

edited

Loading

jaresty commented Sep 3, 2024 •

edited

Loading

nriley commented Sep 7, 2024 •

edited

Loading

knausj85 commented Sep 7, 2024

ekuiter commented Sep 7, 2024

porting the .csv files to .talon-list files #1268

porting the .csv files to .talon-list files #1268

Conversation

saidelike commented Sep 30, 2023

nriley commented Oct 14, 2023 • edited Loading

knausj85 commented Jan 22, 2024 • edited Loading

knausj85 commented Jan 27, 2024

pokey left a comment

Choose a reason for hiding this comment

knausj85 commented Aug 11, 2024

nriley commented Aug 31, 2024 • edited Loading

knausj85 left a comment

Choose a reason for hiding this comment

ekuiter commented Sep 3, 2024

nriley commented Sep 3, 2024

jaresty commented Sep 3, 2024

nriley commented Sep 3, 2024 via email • edited Loading

jaresty commented Sep 3, 2024 • edited Loading

nriley commented Sep 7, 2024 • edited Loading

knausj85 commented Sep 7, 2024

ekuiter commented Sep 7, 2024

nriley commented Oct 14, 2023 •

edited

Loading

knausj85 commented Jan 22, 2024 •

edited

Loading

nriley commented Aug 31, 2024 •

edited

Loading

nriley commented Sep 3, 2024 via email •

edited

Loading

jaresty commented Sep 3, 2024 •

edited

Loading

nriley commented Sep 7, 2024 •

edited

Loading