talonhub / community

Voice command set for Talon, community-supported.
MIT License
618 stars 767 forks source link

Conflict betwen line_commands.talon and comment.talon #298

Closed nriley closed 3 years ago

nriley commented 3 years ago

When in an editor with a programming language tag active, I can't trigger comment [line] <number> from line_commands.talon; Talon always prefers comment from comment.talon and inserts the number in my text instead (e.g., inserting # 24 rather than commenting out line 24).

Seems like there is some overlap between language-specific commenting and editor-specific commenting, both in vocabulary and functionality. Perhaps editor-specific commenting should take precedence if it is implemented, as it's likely to be "smarter" (more context-aware)?

rntz commented 3 years ago

Yeah, I've run into this as well. I've taken to renaming commands (like the ones in line_commands.talon) that comment/decomment lines of existing code freeze (comment) or defrost (uncomment). This solves the problem, but it's a somewhat... idiosyncratic vocabulary choice, maybe not appropriate as a default :P.

I'm not sure there's an easy way to make the editor-specific stuff take precedence? I think we're running into Talon's parsing precedence heuristics here.

dwiel commented 3 years ago

Yeah, I disable the language specific version in my repo. Would be nice to not have to. Can we somehow combine them into one command so without editor support we fall back to text insertion based version?

knausj85 commented 3 years ago

Yeah, I've run into this as well.

My present workaround: I've settled on note/remark for the language-specific things (comment.talon, block_comment.talon), and comment for line_commands.talon to avoid overlap.

Consolidating probably makes sense to explore if someone has the time.

There's no precedence thing yet, save for disabling the entire things via and not tag: user.line_commands or something

nriley commented 3 years ago

A similar issue exists betwen generic_editor.talon and line_commands.talon with the clear/copy/cut/select line commands.

I've worked around this by anchoring the "this line" commands as I realize I never chain them. Others may not find this as useful…

knausj85 commented 3 years ago

I think I changed these commands for different reasons.

You guys don't have anything custom that's similar to this issue, right? https://github.com/knausj85/knausj_talon/commit/26b8f281714a34a2ae605111b8b068a8fe6e4d9a

The commands work most of the time for me, but there is still an occasional conflict with comment <user.text>$ that's worth resolving... somehow.

knausj85 commented 3 years ago

Actually, I think we may have re-introduced the issue with this commit:

https://github.com/knausj85/knausj_talon/commit/646f08d608f871a53f65c360a14f70a7e216aa32#diff-5159ef55fd7e607e8bcac09257cdaf6428c9b4f417721f30f6c0a2c8bfae40c2

For those that are reliably seeing this issue, can you try adding a prefix to the above, or maybe switching the line commands to use <user.number_string> and let me know if it "fixes" the issue?

I think it's a fight between these three commands, just like before

comment line: user.something()
<user.number_string>: "{number_string}"
comment line <number>: user.something_else()

we could potentially standardize on <user.number_string> for these commands.

nriley commented 3 years ago

I just removed my workaround above and I can't replicate any of these issues any more.

Did a bug get fixed in Talon, or am I just somehow managing to trigger the .talon files to be loaded in a different order so it doesn't cause the problem? Is this happening for anyone else using the latest beta?

knausj85 commented 3 years ago

The potentially relevant bug has not been fixed yet - https://github.com/talonvoice/beta/issues/90

We might technically get away with this now: <number> is actually using the same rule under the hood as <number_string>, whereas <digit> was a strict subset?

@mod.capture(rule=f"{number_word}+ (and {number_word}+)*")
def number_string(m) -> str:
    """Parses a number phrase, returning that number as a string."""
    return parse_number(list(m))

@ctx.capture("number", rule="<user.number_string>")
def number(m) -> int:
    """Parses a number phrase, returning it as an integer."""
    return int(m.number_string)
knausj85 commented 3 years ago

This appears to be fixed in the latest beta, but I'll leave it open until others can give it a try.

knausj85 commented 3 years ago

While this seems resolved by the talon 0.1.3 release, I'm thinking it might be beneficial to default to something shorter, and that doesn't conflict with 'comma' from time to time.

maybe note?

rntz commented 3 years ago

Are you suggesting replacing all comment commands with note or just some of them, and if so which: the line_commands ones or the other ones?

I'm inclined to prefer remark over note on the principle of avoiding single-syllable commands (prone to misrecognition or clashes), but it's not a strong preference.

knausj85 commented 3 years ago

I was thinking to replace all of them. Of course, you're right re: single-syllable - good call.