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

Handing / Behaviour of command via -x option / prefs in terminal spawn_child and cross check shell extraction behaviour #829

Open
vssdeo opened this issue Sep 23, 2023 · 0 comments

Comments

@vssdeo
Copy link
Contributor

vssdeo commented Sep 23, 2023

Ref: working directory feature is broken #760

In terminator this is to document and cross-check the behavior of -x which is to execute command and also via preferences / config. These commands eventually end up in terminal.py def spawn_child. Apart from the notes below (to check for shell extraction, where args 0 is reused as shell and command) the window terminates after execution. In worst or best case to document our understanding of this code flow.

  • Default case check done on 2.1.3 and master
  • Test case done on: branch

A) (Default Case) Calling spawn_child in terminal.py via -x cmd line

  1. cmd line:
terminator -d -x find /

in logs:
Terminal::spawn_child: Forking shell: "/usr/bin/find" with args: ['find', '/']

after this the line is args.insert(0, shell) so actual args that reach the terminal:
                      --------------------
   ['/usr/bin/find', 'find', '/']

so -x expects a shell as first argument

  1. Even with
terminator -d -x zsh find / the process dies

   Forking shell: "/usr/bin/zsh" with args: ['zsh', 'find', '/']

   after this the line is args.insert(0, shell) so actual args that reach the terminal:
                          --------------------
       ['/usr/bin/zsh', 'zsh', 'find', '/']
  1. Another command ls
terminator -d -x ls
Terminal::spawn_child: Forking shell: "/usr/bin/ls" with args: ['ls']

same as above the acutal args becomes:
    ['/usr/bin/ls', 'ls]

  1. now calling spawn_child in terminal.py via config/prefs (right-click)

[layouts]
  [[default]]
    [[[window0]]]
      type = Window
      parent = ""
    [[[child1]]]
      type = Terminal
      parent = window0
      profile = default
      command = ls -l /var
      directory = /var

Terminal::spawn_child: Forking shell: "/usr/bin/zsh" with args: ['/usr/bin/zsh', '-c', 'ls -l /var']
....
Terminal::close: close: killing 37947
Terminal::close: os.kill failed: [Errno 3] No such process

In here the command goes in args like:

['/usr/bin/zsh', '/usr/bin/zsh', '-c', 'ls -l /var']


So after above behaviour if we test for lets say ./terminator -d -x zsh find /

the command goes as shown here, which runs find without '/' and print dir list of one dir and not 'find /' which takes some time to run.

Terminal::spawn_child: Before Forking shell: "/usr/bin/zsh" with args: ['zsh', 'find', '/']
Terminal::spawn_child: After Forking shell: "/usr/bin/zsh" with args: ['/usr/bin/zsh', 'zsh', 'find', '/']

B) After code change using feed_child

basic changes done are feed_child is used and after shell is taken from list command[0], 0 arg is not used again to form the full command.

(after change case)

  1. cmd: ./terminator -d -x zsh find / passes on the command find / via feed_child
  • Note shell is expected as first args in code else find is taken as shell and then / is passed to term if shell (zsh) is not given. (this case after change)
    - final args: ['/usr/bin/find', '/']
    - in code it is args = command[1:] because command[0] is taken as shell
    - this works naturally when a shell is given, and diff when not
    so: ./terminator -x top works, but kills window if you quit prog
    ./terminator -x zsh top also works, won't kill window if you quit prog

              but commands like find
              ./terminator -x find /  does not work as find is taken as shell and / as command
              ./terminator -x zsh find / works
    

(previous case without feed_child)
If shell was not passed taking first args twice didn't seem logical. like in previous case (1)
- final args:['/usr/bin/find', 'find', '/']
- in code it was args = command even when command[0] is taken as shell
- this works differently if shell is given
so: ./terminator -x top works
./terminator -x zsh top even though in code shell is expected as first argument.

            but commands like find
            ./terminator -x find /  works as find is taken as shell and also as command and / as command
            ./terminator -x zsh find / does not work.
  1. now calling spawn_child in terminal.py via config/prefs
[layouts]
  [[default]]
    [[[window0]]]
      type = Window
      parent = ""
    [[[child1]]]
      type = Terminal
      parent = window0
      profile = default
      command = ls -l /var
      directory = /var

ls -l /var is passed to the terminal and works normally

Main questions:

  1. so in new case with code change for commands like -x find / , should we take find as shell and command also or should we be consistent expecting first argument as shell or we should check if first arg is not in shell list then we insert a shell ? in this case checking arg 0 or shell, entire thing would be consistent.

  2. using second case keeps the window after command for user to continue

  3. In current case the command is taken as shell and also command in -x (seems incorrect)

  4. if this has to be ignored and custom cmd plugin used as in working directory feature is broken #760 then what happens to cwd and directory in config

vssdeo added a commit to vssdeo/terminator that referenced this issue Sep 23, 2023
…minal spawn_child and cross check shell extraction behaviour gnome-terminator#829

- changed spawn_child behaviour to take commands via feed_child keeping window alive
- changed shell extraction behaviour
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

No branches or pull requests

1 participant