jbuchermn / newm

Wayland compositor
MIT License
959 stars 30 forks source link

rewrite newm-cmd #89

Closed CRAG666 closed 2 years ago

CRAG666 commented 2 years ago

I rewrote newm-cmd, in order to make it behave like a common terminal tool (showing help, error detection, suggestions). Any suggestions are welcome, I also want to modify newm-panel-basic, but I still have to read how it works, if you want to add another command add the respective subcommand in the command_without_args or command_needs_args variables, depending if it needs arguments or not.

command_without_args: dict[str, str] = {
    "inhibit-idle": "Prevents newm from going into idle states (dimming the screen)",
    "config": "Reloads the configuration",
    "lock": "Locks the screen",
    "clean": "Removes orphaned states, which can happen, but shouldn't (if you encounter the need for this, please file a bug)",
    "debug": "Prints out some debug info on the current state of views",
    "finish-inhibit-idle": "Restore the idle states",
    "close-launcher": "Close launcher",
    "unlock": "Unlocks the screen",
}

command_needs_args: dict[str, dict[str, Union[str, dict[str, str]]]] = {
    "open-virtual-output": {
        "help": "opens a new virtual output (see newm-sidecar)",
        "args": {"name": "newm-cmd open-virtual-output <name>"},
    },
    "close-virtual-output": {
        "help": "close a virtual output",
        "args": {"name": "newm-cmd close-virtual-output <name>"},
    },
}
jbuchermn commented 2 years ago

That looks really great, improving newm-cmd is something we should do.

Just two minor thoughts that popped into my head:

CRAG666 commented 2 years ago

That might work, I'll commit and try this

CRAG666 commented 2 years ago

@jbuchermn I already made the change with the suggestions you gave me. Now it only remains for me to review what I did, any suggestion is welcome

CRAG666 commented 2 years ago

These are the commands you exposed in newm-cmd, In the future, I could add/remove commands in this dictionary:

commands: dict[str, dict[str, Union[str, dict[str, str]]]] = {
    "inhibit-idle": {
        "help": "Prevents newm from going into idle states (dimming the screen)",
        "args": {},
    },
    "config": {
        "help": "Reloads the configuration",
        "args": {},
    },
    "lock": {
        "help": "Locks the screen",
        "args": {},
    },
    "clean": {
        "help": "Removes orphaned states, which can happen, but shouldn't (if you encounter the need for this, please file a bug)",
        "args": {},
    },
    "debug": {
        "help": "Prints out some debug info on the current state of views",
        "args": {},
    },
    "unlock": {
        "help": "Unlocks the screen",
        "args": {},
    },
    "open-virtual-output": {
        "help": "opens a new virtual output (see newm-sidecar)",
        "args": {"output_name": "newm-cmd open-virtual-output <name>"},
    },
    "close-virtual-output": {
        "help": "close a virtual output",
        "args": {"output_name": "newm-cmd close-virtual-output <name>"},
    },
}
CRAG666 commented 2 years ago

@jbuchermn is the args parameter a tuple or a dictionary? I ask because I want to include in this pr the deletion of lock-pre and lock-post, but I also wanted to send parameters to lock

    def command(self, cmd: str, arg: Optional[str] = None) -> Optional[str]:
jbuchermn commented 2 years ago

These are the commands you exposed in newm-cmd, In the future, I could add/remove commands in this dictionary:

commands: dict[str, dict[str, Union[str, dict[str, str]]]] = {
    "inhibit-idle": {
        "help": "Prevents newm from going into idle states (dimming the screen)",
        "args": {},
    },
    "config": {
        "help": "Reloads the configuration",
        "args": {},
    },
    "lock": {
        "help": "Locks the screen",
        "args": {},
    },
    "clean": {
        "help": "Removes orphaned states, which can happen, but shouldn't (if you encounter the need for this, please file a bug)",
        "args": {},
    },
    "debug": {
        "help": "Prints out some debug info on the current state of views",
        "args": {},
    },
    "unlock": {
        "help": "Unlocks the screen",
        "args": {},
    },
    "open-virtual-output": {
        "help": "opens a new virtual output (see newm-sidecar)",
        "args": {"output_name": "newm-cmd open-virtual-output <name>"},
    },
    "close-virtual-output": {
        "help": "close a virtual output",
        "args": {"output_name": "newm-cmd close-virtual-output <name>"},
    },
}

This looks really good - in my personal opinion a lot cleaner than before.

Could you please add newm-cmd launcher <app>?

@jbuchermn is the args parameter a tuple or a dictionary? I ask because I want to include in this pr the deletion of lock-pre and lock-post, but I also wanted to send parameters to lock

    def command(self, cmd: str, arg: Optional[str] = None) -> Optional[str]:

You mean in this method? It's just a string. See cmd.py for the relevant line:

result = send_dbus_command({'cmd': command, 'arg': " ".join(args)})

The args tuple passed to the cmd method is joined to one string, passed via dbus, and then passed to Layout.command as the second argument.

CRAG666 commented 2 years ago

@jbuchermn Including newm-cmd launcher seems sensible to me, I'm working on that right now. Second, as for ensure_locked, it receives keyword, so how did this happen, that is, from what I saw, it receives key parameters value something like ensure_locked(dim=True). How I do this?

jbuchermn commented 2 years ago

@jbuchermn Including newm-cmd launcher seems sensible to me, I'm working on that right now. Second, as for ensure_locked, it receives keyword, so how did this happen, that is, from what I saw, it receives key parameters value something like ensure_locked(dim=True). How I do this?

You mean, we should implement newm-cmd lock dim and newm-cmd lock (or similar) to give users the ability to either dim or not? I think that's a good idea. Wen can just do ensure_locked(arg == "dim")

CRAG666 commented 2 years ago

@jbuchermn Including newm-cmd launcher seems sensible to me, I'm working on that right now. Second, as for ensure_locked, it receives keyword, so how did this happen, that is, from what I saw, it receives key parameters value something like ensure_locked(dim=True). How I do this?

You mean, we should implement newm-cmd lock dim and newm-cmd lock (or similar) to give users the ability to either dim or not? I think that's a good idea. Wen can just do ensure_locked(arg == "dim")

Yesterday I realized this. You're right. Everything is already implemented. I hope and you can accept the pr

CRAG666 commented 2 years ago

@jbuchermn Any changes that I should add? or else it only remains to merge.

jbuchermn commented 2 years ago

Nice work, thank you! :)

jbuchermn commented 2 years ago

Just fyi - I added a commit fixing the typing and adding "update-config" command

CRAG666 commented 2 years ago

@jbuchermn Good to see this is already merged. That change seems more sensible to me