Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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