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.

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

New command line option --commands= for multiple commands #98

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ZjYwMj
Copy link

@ZjYwMj ZjYwMj commented Jul 24, 2021

Adds a new --commands= command line option. Note the s in commands.
With the new command line option of --commands=, one can run multiple commands when starting lxterminal from the command line. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes has terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:

  1. src/lxterminal.h
  2. src/lxterminal.c
  3. man/lxterminal.xml

The modification of lxterminal.xml, beside describing the new --commands= option, also describes, in some length, some pitfalls of lxterminal.

Lightly tested.

…d line

Adds a new --commands= command line option. Note the s in commands.
With the new command line option of --commands=, one can run multiple commands when starting lxterminal from the command line. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes has terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:
1. src/lxterminal.h
2. src/lxterminal.c
3. man/lxterminal.xml

The modification of lxterminal.xml, beside describing the new --commands= option, also describes, in some length, some pitfalls of lxterminal.
…d line

Adds a new --commands= command line option. Note the s in commands.
With the new command line option of --commands=, one can run multiple commands when starting lxterminal from the command line. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes has terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:
1. src/lxterminal.h
2. src/lxterminal.c
3. man/lxterminal.xml

The modification of lxterminal.xml, beside describing the new --commands= option, also describes, in some length, some pitfalls of lxterminal.
…and line

Adds a new --commands= command line option. Note the s in commands.
With the new command line option of --commands=, one can run multiple commands when starting lxterminal from the command line. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes has terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:
1. src/lxterminal.h
2. src/lxterminal.c
3. man/lxterminal.xml

The modification of lxterminal.xml, beside describing the new --commands= option, also describes, in some length, some pitfalls of lxterminal.
@FinboySlick
Copy link
Contributor

How does this handle commands that contain a coma? For tab titles, we didn't check too hard because at worst you ended up with an extra tab in the rare event that a title might contain a coma.

In this case however, you may end up with a partially-run command which can potentially be dangerous.

Another question is how complicated will this be when we are pretty much forced to use asynchronous shell spawning - vte_spawn_async()?

@ZjYwMj
Copy link
Author

ZjYwMj commented Jul 25, 2021

  1. As my modified lxterminal.xml says. With my current code, commands are not allowed to have commas. I don't know how serious this limitation is. A partial command is more likely to exit immediately, which will makes the tab closed. Like any other short lived process. It is most likely that the command will not run at all. Are you willing to create a special small lxterminal command language in order to bypass this limitation, with some sort of escape character? Or, alternatively, create another command line switch, to let the user choose the commands separator character? Or something else? If the user have a single command that requires the comma character, he can still use the --command= (without the s) command line option.
    If you insist, will you accept --commands=<user chosen character>,mc<that user chosen character>mc<that user chosen character> ? I mean, the first field can be a single character, followed by a comma, so that in the following commands the separator will be the user chosen character?
  2. I think this will not complicate the transfer to vte_spawn_async() because, by the current code, it doesn't stand in the way of the lxterminal <> vte path. At least at the beginning, and perhaps also after wards, it could be that vte_spawn_sync() will come at the place of vte_terminal_spawn_sync() or vte_terminal_fork_command_full(). And you currently have only one such command in the code.

@FinboySlick
Copy link
Contributor

1- It would be better not to introduce arbitrary limitations or special syntax if we can avoid it. I think the 'right' way to do this would be to allow for multiple --command parameters instead of trying to lump all the commands in a single --commands with separators.
2- vte_spawn_async() will mean spawning the terminals/tabs via a callback, which may or may not maintain the order of the commands. There could be implications to this and we'll have to remember to be explicit about the fact that we cannot guarantee order if this turns out to be the case.

@ZjYwMj
Copy link
Author

ZjYwMj commented Jul 28, 2021

  1. Doesn't --command, without the = character, required to be the last command line option? I guess you meant --command=, with the = character. I can see pros and cons for multiple --command=, as well as for --commands= (with an s) that let the user change the separator character. In my opinion, no alternative has a winning argument. In any case, I will modify the code to accept multiple --command= options. And drop the suggested new --commands= option. It will take me some time to do that.
  2. I haven't delve into the details of vte_spawn_async(). In any case, you already have struct _term to Representative of a tab within a toplevel window in src/lxterminal.h. If the information there is not sufficient to pair each tab with its related call back, I believe variables could be added to obtain such a pairing. But, as stated above, I am only waving my hands. I didn't delve into vte_spawn_async() details.

ZjYwMj added a commit to ZjYwMj/lxterminal that referenced this pull request Aug 15, 2021
… from the command line

The reader is kindly asked to pay attention for the s letter, or absent of, and to the = character, or absent of, in the seemingly no difference --command, --command= and --commands= terms. In what follows, these 3 different terms are totally different. 
With the current stable implementation, with multiple --command= command line options, only the last one was used. It override the previous occurrences of --command=. As promised at lxde#98 (comment), this code modifies that. With it, all the commands specified with --command= option will run. Each one at a different tab. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes have terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:
1. src/lxterminal.h
2. src/lxterminal.c
3. man/lxterminal.xml

Lightly tested.

The modification of lxterminal.xml, beside describing the new behavior of the --command= option, also describes, in some length, some pitfalls of lxterminal.
ZjYwMj added a commit to ZjYwMj/lxterminal that referenced this pull request Aug 15, 2021
… from the command line

The reader is kindly asked to pay attention for the s letter, or absent of, and to the = character, or absent of, in the seemingly no difference --command, --command= and --commands= terms. In what follows, these 3 different terms are totally different. 
With the current stable implementation, with multiple --command= command line options, only the last one was used. It override the previous ocurences of --command=. As promised at lxde#98 (comment), this code modifies that. With it, all the commands specified with --command= option will run. Each one at a different tab. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes have terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:
1. src/lxterminal.h
2. src/lxterminal.c
3. man/lxterminal.xml

Lightly tested.

The modification of lxterminal.xml, beside describing the new behavior of the --command= option, also describes, in some length, some pitfalls of lxterminal.
ZjYwMj added a commit to ZjYwMj/lxterminal that referenced this pull request Aug 15, 2021
…ds from the command line

The reader is kindly asked to pay attention for the s letter, or absent of, and to the = character, or absent of, in the seemingly no difference --command, --command= and --commands= terms. In what follows, these 3 different terms are totally different. 
With the current stable implementation, with multiple --command= command line options, only the last one was used. It override the previous occurrences of --command=. As promised at lxde#98 (comment), this code modifies that. With it, all the commands specified with --command= option will run. Each one at a different tab. Each command is automatically paired with a tab. After exhausting existing tabs, new tabs will be automatically created.

This feature does not lift the basic limitation of commands in lxterminal command line. Which is, that short lived processes makes the tab they ran within, or the whole window in case of only short lived processes, to get closed as soon as the short lived processes have terminated. This limitation is shortly described in the modified man page.

This patch constitutes of modifying 3 files:
1. src/lxterminal.h
2. src/lxterminal.c
3. man/lxterminal.xml

Lightly tested.

The modification of lxterminal.xml, beside describing the new behavior of the --command= option, also describes, in some length, some pitfalls of lxterminal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants