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

Update settings descriptions #1371

Merged
merged 6 commits into from
Jan 27, 2024
Merged

Update settings descriptions #1371

merged 6 commits into from
Jan 27, 2024

Conversation

brief
Copy link
Contributor

@brief brief commented Jan 22, 2024

Suggested update to make settings descriptions clearer and more consistent.

@JWaldenback
Copy link

I just want to pop in and say that I'm digging your "tidying up Talon" pull requests. It's long overdue and I'm happy someone's taking the time!

pokey
pokey reviewed Jan 23, 2024
View reviewed changes
Copy link
Collaborator

@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.

nice. left a few minor comments

README.md Show resolved Hide resolved
settings.talon Outdated Show resolved Hide resolved
settings.talon Outdated Show resolved Hide resolved
settings.talon Outdated Show resolved Hide resolved
@brief
Copy link
Contributor Author

brief commented Jan 24, 2024

Changes made.

Something else I noticed: user.context_sensitive_dictation and user.window_snap_screen are commented out despite being core capabilities with default settings. Does it make sense to uncomment them and set the correct defaults?

This differs from user.paste_to_insert_threshold and user.snippets_dir, which both enable functionality if uncommented.

@JWaldenback
Copy link

To me, it makes perfect sense. Will be easier for new users to understand if it's consistent

pokey
pokey reviewed Jan 25, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@pokey
Copy link
Collaborator

pokey commented Jan 25, 2024 •
edited

Something else I noticed: user.context_sensitive_dictation and user.window_snap_screen are commented out despite being core capabilities with default settings. Does it make sense to uncomment them and set the correct defaults?

This differs from user.paste_to_insert_threshold and user.snippets_dir, which both enable functionality if uncommented.

I guess the consistent thing to do would be:

  • Leave the line uncommented if it differs from default
  • If it is equal to default, change it to not be equal to default, and comment it out

But this PR is a step in the right direction so I'd be happy to accept it as is (modulo final nit above)

@brief @pokey
Update README.md
385364d 
Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
fidgetingbits
fidgetingbits approved these changes Jan 27, 2024
View reviewed changes
Copy link
Collaborator

@fidgetingbits fidgetingbits left a comment

Choose a reason for hiding this comment

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

lgtm

@pokey pokey merged commit f231f62 into talonhub:main Jan 27, 2024
2 checks passed
@brief brief deleted the settings-cleanup branch January 27, 2024 18:48
sisi-sh added a commit to sisi-sh/sisi_talon that referenced this pull request Apr 1, 2024
@sisi-sh
Merge branch 'main' of github.com:talonhub/community
f150dff 
* 'main' of github.com:talonhub/community: (73 commits)
  add lowercased firefox-esr to firefox.py (talonhub#1410)
  configurable number prefix without mouse grid (talonhub#1342)
  Update app.exe matching to work with both public and beta Talon (talonhub#1385)
  Add comment line to javascript (talonhub#1387)
  Add detail to documentation (talonhub#1380)
  Switch to black pre-commit mirror (talonhub#1378)
  Update app_name_overrides.windows.csv to use Windows Terminal instead of iTerm2 (talonhub#1379)
  [pre-commit.ci] pre-commit autoupdate (talonhub#1376)
  coalesce some list updates (talonhub#1348)
  Remove unused setting from c code language (talonhub#1368)
  mouse: adding synonyms for starting left drag. (talonhub#1356)
  Adds commands to focus on the last application (talonhub#1336)
  Stop using “brackets” to mean “braces” (talonhub#1344)
  Update settings descriptions (talonhub#1371)
  Revert changes to `user.mouse_enable_pop_click` (talonhub#1370)
  Fix typo in `mouse.py` variable name (talonhub#1372)
  Convert boolean settings to boolean (talonhub#1360)
  Add ruby op-or-equals command (talonhub#1347)
  added app name Mate-terminal (fork of Gnome-terminal) (talonhub#1367)
  added app name Caja (fork of Nautilus) (talonhub#1366)
  ...
emragins pushed a commit to emragins/talon-community that referenced this pull request May 28, 2024
@brief @emragins
Update settings descriptions (talonhub#1371)
e74d9bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pokey pokey pokey left review comments

@fidgetingbits fidgetingbits fidgetingbits approved these changes

@knausj85 knausj85 knausj85 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@brief @JWaldenback @pokey @fidgetingbits @knausj85