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

Support for new "-t" option (create new tab) #478

Merged
merged 4 commits into from
Jan 29, 2020
Merged

Support for new "-t" option (create new tab) #478

merged 4 commits into from
Jan 29, 2020

Conversation

jakubklos77
Copy link
Contributor

With the new behavior of the terminal trying to match existing tab's path it is impossible to open a new tab with such existing path (working directory option -w).

  • Suppose you have the terminal app open with the tab of location /tmp (running some command or simply waiting)
  • Now you want to open another instance of the same path using:
io.elementary.terminal -w /tmp

This will simply activate the existing tab and nothing else will happen. Hence we cannot achieve the desired resuilt.

A new "-t" option to force a tab creation is the right solution (just lke we have for a new window).
This PR introduces the new option.

Newly, you can open the terminal with:

io.elementary.terminal -t -w /tmp

@jakubklos77 jakubklos77 requested a review from jeremypw January 28, 2020 09:13
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

This does not add a new tab if there is no explicit working directory.
I think you need to change MainWindow.vala line 281 to

            if (directory == null || directory == "" || create_new_tab) {

We also need to open a new tab if there is a command set and if we have
create_new_tab
@jakubklos77
Copy link
Contributor Author

jakubklos77 commented Jan 29, 2020

This does not add a new tab if there is no explicit working directory.
I think you need to change MainWindow.vala line 281 to

            if (directory == null || directory == "" || create_new_tab) {

You are right but the solutionn should be different. See my commit
00350b8. I think this is the right way

if (directory == null || directory == "") {
                if (notebook.tabs.first () == null || command != null || create_new_tab) { //Ensure at least one tab
                    new_tab ("", command);
                }

General conditon for empty directory remains but the new_tab conditon is extended for

  • existing command - this way it will be executed as well
  • create_new_tab

@jakubklos77 jakubklos77 requested a review from jeremypw January 29, 2020 07:56
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

@jeremypw jeremypw merged commit 3918984 into elementary:master Jan 29, 2020
@jakubklos77 jakubklos77 deleted the new_tab_option branch January 29, 2020 11:11
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