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

Implement window cloning ability #24

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Conversation

jzbor
Copy link
Contributor

@jzbor jzbor commented Apr 10, 2024

This implements window cloning as requested in #23.

@orhun
Copy link
Owner

orhun commented Apr 12, 2024

I'm always getting the following log message (with/without -w option)

Unable to set working dir '(null)' for new terminal
Unable to set working dir '(null)' for new terminal
Unable to set working dir '(null)' for new terminal
Unable to set working dir '(null)' for new terminal

Also the working directory does not seem to be set on the new terminal :/ How can we debug this?

@jzbor
Copy link
Contributor Author

jzbor commented Apr 12, 2024

Is it possible that your shell does not include /etc/profile.d/vte.sh. Iirc it is required to work correctly. I can also document this, although I would prefer doing it in a subsequent PR, to avoid additional rebasing.

@orhun
Copy link
Owner

orhun commented Apr 12, 2024

I sourced it (/etc/profile.d/vte.sh) but it doesn't seem to launch from the current working directory:

cap

As you can see, I changed directory to ~/gh and cloned the terminal. But the working directory is still ~/gh/kermit - is that the expected behavior?

@orhun
Copy link
Owner

orhun commented Apr 12, 2024

And yes, please submit another PR after this for mentioning vte.sh 🐻

@jzbor
Copy link
Contributor Author

jzbor commented Apr 15, 2024

Can you provide the shell and distro you are using? It seems to be highly dependent on the distro and how vte/the shell are packed. You can find more information in Tilix' documentation.

@orhun
Copy link
Owner

orhun commented Apr 16, 2024

Distro: Arch Linux
Shell: bash
Terminal: Alacritty

You can find more information in Tilix' documentation.

I already sourced the vte script, is there anything else that I can try?

@jzbor
Copy link
Contributor Author

jzbor commented Apr 17, 2024

What is the output of the following commands on your machine?

type __vte_osc7
type __vte_prompt_command
echo $PROMPT_COMMAND

Also it will not work on alacritty, as it is not vte base as far as I know, the vte checks this at evaluation time.

@jzbor
Copy link
Contributor Author

jzbor commented Apr 17, 2024

@orhun
Copy link
Owner

orhun commented Apr 26, 2024

Sorry for the late reply, I re-setup everything to confirm. Still no luck.

type __vte_osc7

__vte_osc7 is a function
__vte_osc7 () 
{ 
    printf "\033]7;file://%s%s\033\\" "${HOSTNAME}" "$(/usr/lib/vte-urlencode-cwd)"
}

type __vte_prompt_command

__vte_prompt_command is a function
__vte_prompt_command () 
{ 
    local pwd='~';
    [ "$PWD" != "$HOME" ] && pwd=${PWD/#$HOME\//\~\/};
    pwd="${pwd//[[:cntrl:]]}";
    printf "\033]0;%s@%s:%s\033\\" "${USER}" "${HOSTNAME%%.*}" "${pwd}";
    __vte_osc7
}

echo $PROMPT_COMMAND

__zoxide_hook;printf "\033]0;%s@%s:%s\007" "${USER}" "${HOSTNAME%%.*}" "${PWD/#$HOME/\~}"

It should definitely be possible on Arch, but I am not sure how to debug this.

I checked out those links but everything seems good. So I'm not sure what else to check.

But I realized something in the logs:

Unable to set working dir 'file://thinkpad/home/orhun' for new terminal

I was in /home/orhun but the file path starts with my hostname here. Shouldn't that be 'file://home/orhun instead?

@jzbor
Copy link
Contributor Author

jzbor commented Apr 29, 2024

Have you tried zsh? Just to verify if it is an issue with the implementation in kermit or with the shell. Must be a login shell though afaik (zsh -l).

I was in /home/orhun but the file path starts with my hostname here. Shouldn't that be 'file://home/orhun instead?

Yes you are right. This also points to a misconfigured PROMPT_COMMAND. Sadly I am not to familiar with bash, as I am currently using zsh and this seems to be a point where the shells differ, so I am not of much help.

I tried reproducing it with my local bash shell and indeed it does not work either. So maybe it is a bash issue?

@jzbor
Copy link
Contributor Author

jzbor commented May 30, 2024

Any updates on this?

@orhun
Copy link
Owner

orhun commented May 31, 2024

I tried it now, unfortunately it didn't work as well. This time the logs were correct but the new terminal is not launched on that directory:

Unable to set working dir 'file:///home/orhun/gh/kermit' for new terminal
Unable to set working dir 'file:///home/orhun/gh' for new terminal
Unable to set working dir 'file:///home/orhun/gh' for new terminal
Unable to set working dir 'file:///home' for new terminal

I sourced the vte.sh in .zshrc, not sure if that is correct. I'm also not sure how to do all of this in a login shell, I simply overrided SHELL environment variable to zsh.

@jzbor
Copy link
Contributor Author

jzbor commented Jun 2, 2024

I have improved the error logging, so it should output a reason for a failed chdir. I don't think this will help with bash, but maybe it provides additional insights on why it does not work with zsh either.

@orhun
Copy link
Owner

orhun commented Jun 2, 2024

This time I get:

Unable to change working directory for new terminal: No such file or directory
Unable to change working directory for new terminal: No such file or directory
Unable to change working directory for new terminal: No such file or directory

@jzbor
Copy link
Contributor Author

jzbor commented Jun 2, 2024

Ok so there was indeed a bug in the strncmp check to remove the prefix. I also found out that it does not work properly if arg[0] is a relative path like ./build/kermit, because as soon as you change the directory this path becomes invalid. However this should not be a problem for actual deployments.

EDIT: I hope it should work properly now at least for zsh.

@orhun
Copy link
Owner

orhun commented Jun 3, 2024

Yup, works for zsh now! 🥳

@orhun
Copy link
Owner

orhun commented Jun 4, 2024

I think this is ready to go. But before that can you submit an issue about the bash issue that we're having with this functionality? I also took a look at this just now and I'm not sure why it doesn't work for me. Maybe someone that knows what's going on will show up and fix this in the future.

@orhun
Copy link
Owner

orhun commented Jun 4, 2024

Changed my mind, you already did a lot for this. I created the issue in #26

Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your contribution and patience with this!

@orhun orhun changed the title Implementing window cloning ability (implementing #23) Implement window cloning ability Jun 4, 2024
@orhun orhun merged commit 2c18861 into orhun:master Jun 4, 2024
1 check passed
@jzbor
Copy link
Contributor Author

jzbor commented Jun 4, 2024

Thanks for reviewing and being open to contributions :)

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