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.

Sign up for GitHub

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

Jump to bottom

Add support for xfce4-terminal #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for xfce4-terminal #7

wants to merge 3 commits into from

Conversation

JakeIsMeh
Copy link

Add support for xfce4-terminal.
Additionally refactored the logic for opening the terminal in a new tab.

Git also caught a trailing space.

@JakeIsMeh
Copy link
Author

Tested with mate-terminal and xfce4-terminal.
OS: Arch Linux w/ Budgie 10.6.4

JoshStrobl
JoshStrobl requested changes Sep 3, 2022
View reviewed changes
Copy link
Member

@JoshStrobl JoshStrobl left a comment

Choose a reason for hiding this comment

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

--tab should only be passed for GNOME Terminal when the file type is File. It does not do anything for directories, hence why the logic was what it was.

@JakeIsMeh JakeIsMeh marked this pull request as draft September 3, 2022 10:34
@JakeIsMeh
Copy link
Author

While testing, I discovered that GNOME Terminal's --tab option is for specifying tabs in a new window, and not for opening a new tab in any current open windows.

@JakeIsMeh JakeIsMeh marked this pull request as ready for review September 3, 2022 10:53
@JakeIsMeh JakeIsMeh requested a review from JoshStrobl September 3, 2022 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshStrobl JoshStrobl Awaiting requested review from JoshStrobl JoshStrobl is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JakeIsMeh @JoshStrobl