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

Minor refactoring for fish clone-in-kitty and allow use without env -0 #4967

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

page-down
Copy link
Contributor

The builtin option from the latest version of fish is not used for now. clone-in-kitty has been tested with fish 3.2.x.
BusyBox env does not support the -0 option. Implemented with fish.

Builtins are heavily used in the scripts that come with fish, and any environment that overrides these builtin, I believe fish will not work properly. No need to guard with builtin, simplify the code for future maintenance.
If we use command, then base64 or other commands that is not in the path, e.g. alias or function, will not work.
If builtin or commands don't work, I think clone-in-kitty is the last thing to worry about, leaving the user to work it out on their own, rather than rejecting the functions from the normal users.

Remove quotation marks and the trailing # ", # ‘ by escaping special characters in the string.

Remove echo $data_len.

Please review, thanks.


I found that environment variables containing newlines, the \n are lost after cloning. This is reproduced in zsh/fish. (I believe the base64-encoded environment variables are correct in fish.)
I need help on this.

Also the zsh and bash versions of clone-in-kitty do not work in environments that do not support env -0.


I personally don't like to copy all environment variables, because the shell is not only environment variables, but also many internal variables (and funtions) that are not exported. This feature only copies half of them, and my environment is broken and missing something (that I know) after cloning. I prefer to explicitly configure the ones that need to be copied.

Or, maybe it's time to increase the robustness of my environment.

The builtin option from the latest version of fish is not used for now.
clone-in-kitty has been tested with fish 3.2.x.
BusyBox env does not support the -0 option.
@kovidgoyal kovidgoyal merged commit afaf866 into kovidgoyal:master Apr 14, 2022
@kovidgoyal
Copy link
Owner

kovidgoyal commented Apr 14, 2022 via email

@page-down page-down deleted the ksi-fish-clone-in-kitty branch April 14, 2022 14:24
@kovidgoyal
Copy link
Owner

Actually I did the following test
export n=$'a\nb'; clone-in-kitty
echo $n (in the cloned window)
and it output
a
b

as expected under zsh. Did you mean over ssh?

@page-down
Copy link
Contributor Author

I tested again and confirmed that local and ssh, zsh clone-in-kitty are working fine.

However, the newline in the environment variable is normal after using local fish clone-in-kitty, but missing with ssh, and the cause has not been found yet.

Also my local zsh clone-in-kitty is throwing an error. (when the macOS user's shell is zsh)
I think it also errors out when the shell in /etc/passwd is changed to bash or fish.

Failed to launch child: -zsh
With error: No such file or directory
Press Enter to exit.

@page-down
Copy link
Contributor Author

I noticed that after using zsh clone-in-kitty, it displays a percentage sign with a white background and black text. Where is this coming from? Is it some kind of "partial line"?

@page-down
Copy link
Contributor Author

Actually I did the following test ... as expected under zsh. Did you mean over ssh?

I tested again and confirmed that local and ssh, zsh clone-in-kitty are working fine.

I remembered how I tested zsh (with ssh) before.

export DEBUG=$(printf "abc\ndef")
env
DEBUG=abc
def

after zsh clone-in-kitty with ssh:

DEBUG=abcdef

@kovidgoyal
Copy link
Owner

yes, with ssh its probably not going to work because of the serialize_env problem
I mentioned before. But, locally it should work fine.

@page-down
Copy link
Contributor Author

I removed \n from quote_pat and it looks normal.
Do you think that's enough?
I wonder if something could go wrong.

# quote_pat = re.compile('([\\`"\n])')
quote_pat = re.compile('([\\`"])')

As for Failed to launch child: -zsh

kitty/child.py

if is_macos and exe == shell_path:
    argv[0] = (f'-{exe.split("/")[-1]}')

kitty/macos_process_info.c cmdline_of_process

Does it require special case handling? Or should we get process information in some other way?

@kovidgoyal
Copy link
Owner

kovidgoyal commented Apr 15, 2022 via email

@kovidgoyal
Copy link
Owner

kovidgoyal commented Apr 15, 2022 via email

@page-down
Copy link
Contributor Author

... Where are you seeing it?

This percent sign appears when clone-in-kitty is run again in a zsh or fish session after clone-in-kitty. zshrc is cleared.
I want to be sure that it works properly in various situations.

In the recent commit there is a mention of a change related to macOS, I tried it and still got Failed to launch child: -zsh.

@kovidgoyal
Copy link
Owner

I tried running clone-in-kitty, then clone-in-kitty again in the cloned window and saw no %. Also with empty .zshrc

I dont have access to my macos machine to test at the moment, but it should be easy to verify whats going on. See line 590 in launch.py

@page-down
Copy link
Contributor Author

... saw no %. Also with empty .zshrc

I specifically tried it in a Linux environment and kitty -o shell=/bin/zsh ran clone-in-kitty (first time) and the percent sign came up. I don't know the root cause yet until I dig deeper.

... it should be easy to verify whats going on ...

It is, only some minor changes are needed.

kitty --config=NONE

cmdline: ['-zsh']
window.child.argv[0]: /bin/zsh
window.child.final_exe: /bin/zsh

@kovidgoyal
Copy link
Owner

kovidgoyal commented Apr 15, 2022 via email

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