talonhub / community

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

establish generic_terminal.talon and deprecate terminal.talon (both for linux and mac) #505

Closed DonnerWolfBach closed 3 years ago

DonnerWolfBach commented 3 years ago

As far as I can see, a generic talon grammar like in the guidelines for contributions should be used whenever possible. And while there is generic_terminal, it is not really used. The terminals on linux use the terminal tag and there's a seperate and mostly redundant apple_terminal. I discussed briefly with knausj whether a refactor would be necessary and it seems like it is.

My proposal:

  1. establish generic_terminal in the guidelines for contributions
  2. deprecate terminal.talon (both the linux and the mac version) and move to generic_terminal
  3. For operating system specific commands (like maybe "resume" in apple_terminal) create files like apple_terminal or linux_terminal
knausj85 commented 3 years ago

This sounds reasonable, thanks. I would note generic_terminal is used for most of terminal support on Windows, which may or may be useful for adding support.

fidgetingbits commented 3 years ago

I agree that we should move to generic_terminal.talon and we can get rid of terminal.talon, but i don't entirely understand why does this would involve removing the terminal tag (mentioned in your title, but not in the proposal)? Other things like the generic_browser.talon also have a browser tag for instance. Something needs to assert the terminal tag in order to trigger generic_terminal.talon anyway no?

There's other reasons to have the tag too, for instance in my repo i have the idea of generic command line package managers, and you can assert which one you're using based off of a tag that's asserted in generic_packager.talon, but for instance there generic_packager.talon only comes into context if terminal tag is asserted. Furthermore i have specific command line rules that are only relevant if running inside of a terminal, that aren't suited to generic_terminal.talon. You can see here. I'm pretty sure this isn't the only thing that works like that, although i'm not sure if there's anything else in knausj atm that does it.

I think from your discussion on slack it seemed like you mostly wanted to get rid of the terminal tag from git.talon because it broke the way you were doing generic_terminal.talon? I agree you could just assert a user.git tag inside of generic_terminal.talon and then modify git.talon to not rely on user.terminal tag. But it would be good to hear why it shouldn't be in there and that we make sure that if the tag actually goes away that there's some sort of sane replacement. If there's some better way that you envision doing what i'm doing above without the tag, then i'm also interested to hear. Im guessing you prefer the idea of having generic_terminal.talon just asserting the git tag instead so terminal is implied?

I suppose it's worth noting something i've been planning on doing eventually which is abstracting git commands out in to even more generic commands, so that if you use them source from the existing git.talon it runs the command on the terminal (which would also rely on the terminal tag), but if you ran them inside a vim they would get piped through to the get plug in, for example here, in for that to work you would need to be able to differentiate terminal and vim contexts using the tags. I mention this just to try to give some context as to the type of thinking for why git.talon has the terminal tag check at all.

I appreciate there's no way that you would know how other people are using this tag in other repos, but some of this my stuff will likely make its way back into knausj repo when i finally send my next round of pull requests (famous last words), so something to keep in mind

knausj85 commented 3 years ago

I would note the terminal tag is a Talon-declared tag, and it still makes sense to assert. I could probably be persuaded to get rid of user.generic_terminal and just use terminal if folks think that's a cleaner approach. I'm not really convinced by my own argument for user.generic_terminal now. :-/

I agree the terminal tag as below is also redundant.

tag: terminal
and tag: user.git
fidgetingbits commented 3 years ago

ah right. i might have it redeclared the terminal tag in mine by accident at some point. i agree just using the built in terminal tag, and not having a generic_terminal tag makes sense. I don't think most of the other stuff that uses generic actually has an associated generic_xxx tag, but something like the packager in my repo is a little bit different i guess because it's a subset of the terminal stuff.

I'm fine with getting rid of the terminal tag from git for now too, and i'll just revisit it in the future in case i end up doing the generic git . not to hijack this too much but on that note i'm curious for whatever ide you're typically using, do you typically issue git commands that would be suited for something like a generic rcs style thing and that overlap with what you might be using in the terminal from the git.talon commends?

DonnerWolfBach commented 3 years ago

Ok, so what I see as the (interim) result would be:

About git.talon: I don't have a problem with having to have both the terminal tag and the git tag activated for it. The reason this resulted in this issue was that I realized generic_terminal was not used in the way I expected it to be (and that the two terminal.talon files existed).

DonnerWolfBach commented 3 years ago

About my ide: Mostly vs code (but switching to vs codium), sometimes vim. For git stuff, I always go to the terminal (kde-konsole or powershell in my case).

I see that having a "git interface" with implementations for terminals (I think one for all would be enough, or are there any differences?) and one for each ide (where git is supported like e.g. in vs code) makes sense.

fidgetingbits commented 3 years ago

Sounds good about the interim result.

DonnerWolfBach commented 3 years ago

Ok, I implemented kde-konsole how I envision all terminals to be implemented. Mainly: generic_terminal.talon is now activated by the terminal tag The implementation of the kde-konsole can be found here: https://github.com/DonnerWolfBach/knausj_talon/tree/knausj_improved_kde_konsole/apps/platforms/linux/kde-konsole

I will now begin to add some commands (like rerun search) to generic_terminal from the two terminal.talon files

knausj85 commented 3 years ago

That looks great, @DonnerWolfBach.

I doubt you'll need any of the stuff below, as I believe it was only necessary for some Windows Explorer silliness

user_path = os.path.expanduser("~") # is this important?
directories_to_remap = {} # is this important?
directories_to_exclude = {} # is this important?
DonnerWolfBach commented 3 years ago

At the moment I am a bit unsure whether and where to include the file_manager tag. It was both used in the apple and linux terminal.talon, but the file_manager.talon file is so large that I don't quite get a proper grasp on it. Maybe you know wether it's important or not.

knausj85 commented 3 years ago

The file_manager tag probably belongs in each specific console's talon file (or py file). That way, it's correctly asserted as support is added.

DonnerWolfBach commented 3 years ago

Ok, I removed the terminal.talon for linux since it doesn't contain anything linux specific. I also removed redundant stuff from the terminal.talon for apple. (However, since I don't have something with mac os I can't test it).

Also I wouldn't transition the job control (resume) command suspend since it seems to be a "unixoid" specific thing and not present on windows.

The (hopefully) last thing I'm not sure about is how to deal with terminal.py in linux (since no mac I won't even touch the mac terminal.py). It's very likely that the stuff defined there is valid for ALL terminals in linux, however I am not sure about it. So the question is whether to transition the content of linux/terminal.py to the specific terminals python files or leave it there, hoping there's no terminal it's incompatible with. I prefer moving everything to the terminal specific files (like kde-konsole.py), what do you think?

knausj85 commented 3 years ago

I would err on the side of separate/specific terminal files from experience, but maybe @fidgetingbits may know better as the linux guru.

DonnerWolfBach commented 3 years ago
  1. Changed the issue title to something correct
  2. Added support for powershell (on windows) - which was basically updating and adding to the existing implementation
fidgetingbits commented 3 years ago

I think it probably make sense to setup a PR at this point as a draft so we can see all the changes. But at a glance from what you said it mostly looks good. I guess having it per terminal file probably makes sense for most stuff just because I can't say for sure which terminals support which key sequences. Especially relevant for the tab commands, since not all terminals are going to support tabbing. That said though I think they're certain things that aren't really tied to the terminal itself, but more the shell or whatever, that aren't really suited to be duplicated inside of each terminal file.

For instance to me all of the stuff below is definitely going to be a same across everything any one ever uses, who it seems like it shouldn't be duplicated. These are really shell commands more so than terminal app specific, even if we're going to cover them by a generic_terminal actions.

@ctx.action_class("user")
class user_actions:
    # terminal-tag functions implementation
    def terminal_list_directories():
        actions.insert("ls")
        actions.key("enter")

    def terminal_list_all_directories():
        actions.insert("ls -a")
        actions.key("enter")

    def terminal_change_directory(path: str):
        actions.insert("cd {}".format(path))
        if path:
            actions.key("enter")

    def terminal_change_directory_root():
        actions.insert("cd /")
        actions.key("enter")

    def terminal_clear_screen():
        actions.insert("clear")
        actions.key("enter")

    def terminal_run_last():
        actions.key("up enter")

Side note that these two are actually wrong in terminal.py and should be ctrl-left and ctrl-right:


    def word_left():
        actions.key('ctrl-w left')
    def word_right():
        actions.key('ctrl-w right')
``
DonnerWolfBach commented 3 years ago

Ok, I'll create a new, properly named branch with my changes and create a PR for it.

Unfortunately, "ls -a" doesn't work on windows, "cd /" may not work there as well. Still, the are likely the same among all linux shells. (I may add that I am not familier with differences between shells). So for now, i would still move them to the terminal-specific files, but I can see how this may be "too much".

fidgetingbits commented 3 years ago

Good idea and good point about windows. I guess we probably eventually want something like generic_unix_shell and generic_windows_shell then? Can always do of first past with what you have already, and then migrate the duplication over to a single shell file later if you don't want to deal with it. As far as I know macos uses zsh by default, so there should be complete overlap with the linux for the generic parts, at least like cd, ls, so in that sense we shouldn't need a generic_mac and generic_linux, although if we think that's too confusing we could just allow duplication in there, which imo is still better than duplicating all of the commands for every single different terminal, is at least the duplication is minimal.

fwiw shells like busybox, bash, zsh, sh, csh, etc all support this sort of really baseline standard of command-line utilities/commands, but they all start to have their own little quirks and features that don't overlap. So just having the basics generic I think is a good start and then if/when people run into other shell-specific stuff they can add it into the shell specific file (I already have some stuff for zsh in my repo, as do others)

DonnerWolfBach commented 3 years ago

I checked and cd / does work on windows/powershell. So the only thing from generic_terminal that doesn't work as on linux and mac is "ls -a". However, instead of having to split things up we could do this:

# generic_terminal.py
def terminal_list_all_directories():
        """Lists all directories including hidden"""
        if os.name != "posix":
            actions.insert("ls -force")
        else: 
            actions.insert("ls -a")

        actions.key("enter")

This does feel slightly dirty, but much more pragmatic than splitting things up into unix_shell and windows_shell or directly into the specific terminals. If however more commands which are different between unixoid and windows occur, we could still do the split.

This would now mean we have three "layers":

  1. generic_terminal.py
  2. \<os>_terminal.py
  3. \<specific terminal>.py and \<specific terminal>.talon

I would split the implementation and tag activations up like this:

generic_terminal.py

I think tab should be terminal specific cause they can be configured there differently (though am not 100% convinced). Anything I forgot? If you agree to this proposal, I would implement it and then de-draft the PR.

DonnerWolfBach commented 3 years ago

@knausj85 What do you think about this (as an alternative to the generic_unix_shell tag or talon file (activated by os.name="Posix"))?

knausj85 commented 3 years ago

I've been planning to move away from using app.platform in places to better support VMs and such down the line (#446), and I think os.name would introduce the same problem.

DonnerWolfBach commented 3 years ago

Hm, I was thinking about the "Generic_unix_terminal activated by tags in the specific terminal file" solution and I feel while it is a "clean" solution, it is quite complicated. I think without proper documentation/commands it will take a long time for new people to understand what's going on.

I mean we can surely add the doc comments and reference some part in the Readme, but I feel like this wouldn't be well maintained.

DonnerWolfBach commented 3 years ago

@fidgetingbits @knausj85 I implemented the generix_unix_shell approach here. What do you think? Should I add an README.md in the generic_terminal folder explaining what the idea behind the "terminal architecture" is?

DonnerWolfBach commented 3 years ago

I finalized the architecture for the terminal implementation in the PR #514. The structure is this: generic_terminal.talon lists some commands that should be possible in every terminal, like lisa (list directories). generic_windows_shell and generic_unix_shell.py provide the implementations for these commands (there are major enough differences between windows shells and unixoid shells). Everything else, like tab support, activation of specific commands sets is done in the .talon and .py files for the specific terminals.

At the moment only kde-konsole is fully implemented that way and can be used as a reference.

DonnerWolfBach commented 3 years ago

Since #514 is merged, I'll close this issue. Migrating all the terminals will be tracked by me in #562