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

Fix #74 #75

Merged
merged 5 commits into from
May 18, 2023
Merged

Fix #74 #75

merged 5 commits into from
May 18, 2023

Conversation

Moomboh
Copy link
Contributor

@Moomboh Moomboh commented Apr 28, 2023

Fixes #74

@Moomboh Moomboh changed the title Fix #74 and #55 Fix #74 Apr 28, 2023
@Moomboh
Copy link
Contributor Author

Moomboh commented May 13, 2023

@uranusjr After digging into this again I went the extra mile and also did the refactoring you mentioned here: #55 (comment). This means it fixes #55 and should also fix #35. Additionally, since I wanted to make the ps fallback also tty independent, I thought I might just as well change the ps call to berkeley style for better compatibility and try to address #21 with this as well.
I could not verify if this actually also fixes #21 and #35 though.

@uranusjr
Copy link
Member

Let’s not be greedy, and only fix one issue at a time instead.

@Moomboh
Copy link
Contributor Author

Moomboh commented May 16, 2023

I reverted the changes to the ps call and only made it fit the refactor. This PR now only focuses on fixing #74 and #55 and should not change other behaviour.
@uranusjr Thank you very much for your feedback, so far!

@uranusjr
Copy link
Member

I pushed a commit to refactor the get_process_parents into generator functions and slightly reworded a comment. Can you take a look to ensure I haven’t broken anything at your end?

@Moomboh
Copy link
Contributor Author

Moomboh commented May 18, 2023

Looks good to me. Still correctly detects shell inside docker container on running on M1 MacBook Pro/osx-arm64 both with rosetta and qemu amd64 emulation.

@uranusjr uranusjr merged commit c2b9431 into sarugaku:master May 18, 2023
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.

Fails to detect shell in docker containers run with rosetta on Apple Silicon
2 participants