Merge lp:~jeremywootten/pantheon-terminal/fix-1208140 into lp:~elementary-apps/pantheon-terminal/trunk

[Jeremy Wootten]

Fixed bug #1208140, moved -d to -w, steps necessary taken to fix #1033456 in Files.

1/. Implements the command line option to open terminal at a specified location.

2/. The short option has been changed from -d to -w to avoid conflict with the debug option.

3/. Repeat use of the option opens the location in a new tab on the (last) existing window instead of opening a new window.

4/. This was implemented as part of fixing bug #1033456 in pantheon-files - implementing an "Open Terminal here" option in the context menu. That bug-fix is dependent on this one.

Note: working_directory has been changed from a static to an instance variable in order to permit repeated use of the option. Consequential changes are made to PantheonTerminalWindow.

[David Gomes (davidgomes)]

Why the variable extra_entries?

[Jeremy Wootten (jeremywootten)]

If I include the -w option entry into "entries" then it only works as
expected for the first terminal window, any subsequent window is opened at
the same directory regardless of what directory you specify after -w. This
is because "entries" is parsed in main in a static context so the working
directory it sets must be static.

"extra_entries" is parsed in _command_line and sets an instance variable
"working_directory" so you can repeatedly run the command
"pantheon-terminal -w [directory_path]" and each window opens at the
specified directory.

Is there a better way of doing this?

On 15 August 2013 17:49, David Gomes <email address hidden> wrote:

> Why the variable extra_entries?
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Akshay Shekher (voldyman)]

good work jeremy, one thing is missing

--help doesn't show --working-directly/-w option.

review: Needs Fixing

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

503. By Jeremy Wootten on 2013-08-18

Revised desktop file - only a single one needed

[Jeremy Wootten (jeremywootten)]

The latest revision fixes this problem (and removes the need for the
separate variable "extra-entries")

On 16 August 2013 13:09, Akshay Shekher <email address hidden> wrote:

> Review: Needs Fixing
>
> good work jeremy, one thing is missing
>
> --help doesn't show --working-directly/-w option.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[David Gomes (davidgomes)]

>Name=Pantheon Terminal
Pantheon Terminal is installed under the name of Terminal on purpose.

review: Needs Fixing

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

504. By Jeremy Wootten on 2013-08-24

Correct name in desktop file, correct formatting, remove unnecessary changes

[Jeremy Wootten (jeremywootten)]

OK, fixed that.

On 23 August 2013 19:06, David Gomes <email address hidden> wrote:

> Review: Needs Fixing
>
> >Name=Pantheon Terminal
> Pantheon Terminal is installed under the name of Terminal on purpose.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Cody Garver (codygarver)]

Conflicts diff lines 161, 165, 168

review: Needs Fixing

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

505. By Jeremy Wootten on 2013-08-24

Merged changes from trunk revision 507

[Jeremy Wootten (jeremywootten)]

Sorry - now merged trunk up to revision 507 and resolved conflicts.

On 24 August 2013 08:00, Cody Garver <email address hidden> wrote:

> Review: Needs Fixing
>
> Conflicts diff lines 161, 165, 168
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Sergey "Shnatsel" Davidoff (shnatsel)]

It's better to make a new .desktop file called "Open Terminal Here" with "NoDisplay=True" key instead of changing the existing .desktop file. This will make "Terminal" show up in app launchers (e.g. Slingshot) and "Open Terminal Here" in Open With menus.

So I'm adding my name to the long list of "Needs Fixing" reviewers :D
But thanks for tackling this issue!

review: Needs Fixing

[Jeremy Wootten (jeremywootten)]

Thanks for the advice, I will do that.

On 27 August 2013 15:26, Sergey "Shnatsel" Davidoff <email address hidden>wrote:

> Review: Needs Fixing
>
> It's better to make a new .desktop file called "Open Terminal Here" with
> "NoDisplay=True" key instead of changing the existing .desktop file. This
> will make "Terminal" show up in app launchers (e.g. Slingshot) and "Open
> Terminal Here" in Open With menus.
>
> So I'm adding my name to the long list of "Needs Fixing" reviewers :D
> But thanks for tackling this issue!
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

506. By Jeremy Wootten on 2013-08-28

Use separate open-terminal-here.desktop file

[Jeremy Wootten (jeremywootten)]

I have uploaded a revision with a separate open-terminal-here.desktop file
as recommended. I was not quite sure how many keys needed to be repeated
in this file, but it works anyway.

You need to update the ~/.local/share/applications/mimeapps.list file to
stop the Terminal option appearing, of course, if you have already used
that option.

On 27 August 2013 15:26, Sergey "Shnatsel" Davidoff <email address hidden>wrote:

> Review: Needs Fixing
>
> It's better to make a new .desktop file called "Open Terminal Here" with
> "NoDisplay=True" key instead of changing the existing .desktop file. This
> will make "Terminal" show up in app launchers (e.g. Slingshot) and "Open
> Terminal Here" in Open With menus.
>
> So I'm adding my name to the long list of "Needs Fixing" reviewers :D
> But thanks for tackling this issue!
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Sergey "Shnatsel" Davidoff (shnatsel)]

Thanks! The keys in the open-terminal-here.desktop file are fine. I'd prefer to change the name to "open-pantheon-terminal-here.desktop" because the current name can cause collisions with some other terminal emulator which does the same thing.

Also, I get text conflicts in src/PantheonTerminalWindow.vala now, so I can't test the result of the merge.

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

507. By Jeremy Wootten on 2013-09-03

Rename open-terminal-here.desktop to open-pantheon-terminal-here.desktop

508. By Jeremy Wootten on 2013-09-03

merged changes from trunk revision 514

[Jeremy Wootten (jeremywootten)]

I have now renamed the desktop file and merged changes from trunk.

On 2 September 2013 09:22, Sergey "Shnatsel" Davidoff <email address hidden>wrote:

> Thanks! The keys in the open-terminal-here.desktop file are fine. I'd
> prefer to change the name to "open-pantheon-terminal-here.desktop" because
> the current name can cause collisions with some other terminal emulator
> which does the same thing.
>
> Also, I get text conflicts in src/PantheonTerminalWindow.vala now, so I
> can't test the result of the merge.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Sergey "Shnatsel" Davidoff (shnatsel)]

"open-pantheon-terminal-here.desktop" doesn't show up in the list of apps to open a folder with, not even as a recommended application in the "Other application..." dialog.
I've added "inode/directory" mimetype to the "open-pantheon-terminal-here.desktop" (why is it not in there?) and ran "sudo update-mime-database /usr/share/mime/" and restarted pantheon-files but that didn't help. What am I missing?

[Julián Unrrein (junrrein)]

I have the same issue Shnatsel is talking about.

[Julián Unrrein (junrrein)]

@Shnatsel

It seems that you also need to add an "inode/directory=open-pantheon-terminal-here.desktop" entry to /usr/share/applications/mimeinfo.cache. Running "sudo update-mime-database ..." didn't change the file for me.

Unfortunately, doing this doesn't make Terminal show up as a default application.

[Victor Martinez (victored)]

Time to write a contract file for Pantheon Terminal? :)
On Sep 15, 2013 3:52 PM, "Julián Unrrein" <email address hidden> wrote:

> @Shnatsel
>
> It seems that you also need to add an
> "inode/directory=open-pantheon-terminal-here.desktop" entry to
> /usr/share/applications/mimeinfo.cache. Running "sudo update-mime-database
> ..." didn't change the file for me.
>
> Unfortunately, doing this doesn't make Terminal show up as a default
> application.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> Your team elementary Apps team is subscribed to branch
> lp:pantheon-terminal.
>

[Jeremy Wootten (jeremywootten)]

I'll look into this. It may need a change to the install script.

On 15 September 2013 22:52, Julián Unrrein <email address hidden> wrote:

> @Shnatsel
>
> It seems that you also need to add an
> "inode/directory=open-pantheon-terminal-here.desktop" entry to
> /usr/share/applications/mimeinfo.cache. Running "sudo update-mime-database
> ..." didn't change the file for me.
>
> Unfortunately, doing this doesn't make Terminal show up as a default
> application.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Jeremy Wootten (jeremywootten)]

You need to run "sudo update-desktop-database". We have added a new
desktop file, not created a new mime type.

I need to find out how to get cmake to do this automatically on
installation - or is this done by the deb package anyway?

You are right - the mime-type should now be "inode/directory" - I will
change it.

On 15 September 2013 22:04, Sergey "Shnatsel" Davidoff
<email address hidden>wrote:

> "open-pantheon-terminal-here.desktop" doesn't show up in the list of apps
> to open a folder with, not even as a recommended application in the "Other
> application..." dialog.
> I've added "inode/directory" mimetype to the
> "open-pantheon-terminal-here.desktop" (why is it not in there?) and ran
> "sudo update-mime-database /usr/share/mime/" and restarted pantheon-files
> but that didn't help. What am I missing?
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Sergey "Shnatsel" Davidoff (shnatsel)]

It should be done automatically on installation of .deb package, but I'm not 100% sure.
And I believe both mimetypes should be present in the "invisible" .desktop file.

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

509. By Jeremy Wootten on 2013-09-17

change MimeType to inode/directory; run update-desktop-database during install

510. By Jeremy Wootten on 2013-09-17

Merge changes from trunk up to revision 519

[Jeremy Wootten (jeremywootten)]

update-desktop-database complained about the presence of
"x-directory/normal" in the desktop file so I took it out but I can put it
back if you like.

I have just uploaded a new version which runs update-desktop-database
during make install. Does this fix the problem you were having?

On 17 September 2013 19:45, Sergey "Shnatsel" Davidoff
<email address hidden>wrote:

> It should be done automatically on installation of .deb package, but I'm
> not 100% sure.
> And I believe both mimetypes should be present in the "invisible" .desktop
> file.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Cody Garver (codygarver)]

Remove update-desktop-database and restore "x-directory/normal".

The general rule is to not duplicate utilities already run by the packaging.

We ignore the warnings about "x-directory/normal" because there is no known alternative to its functionality.

review: Needs Fixing

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

511. By Jeremy Wootten on 2013-09-19

Remove update-desktop-database from CMakeLists.txt; Add x-directory/normal back to .desktop file.

[Jeremy Wootten (jeremywootten)]

OK, done

On 19 September 2013 13:37, Cody Garver <email address hidden> wrote:

> Review: Needs Fixing
>
> Remove update-desktop-database and restore "x-directory/normal".
>
> The general rule is to not duplicate utilities already run by the
> packaging.
>
> We ignore the warnings about "x-directory/normal" because there is no
> known alternative to its functionality.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Cody Garver (codygarver)]

-d does not work for me

>$ pantheon-terminal -d ~/Downloads

review: Needs Fixing

[Cody Garver (codygarver)]

-w works

[Jeremy Wootten (jeremywootten)]

The option was changed to -w so that it did not clash with the -d (debug)
option of Granite.Application

On 21 September 2013 11:35, Cody Garver <email address hidden> wrote:

> -w works
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[lp:~jeremywootten/pantheon-terminal/fix-1208140]

512. By Jeremy Wootten on 2013-09-22

Pass unrecognized options to Granite.Application

[Jeremy Wootten (jeremywootten)]

I have now fixed it so that unrecognised options are correctly passed onto
Granite.Application, so pantheon-terminal -d will run with the "debug"
option.

On 21 September 2013 11:34, Cody Garver <email address hidden> wrote:

> Review: Needs Fixing
>
> -d does not work for me
>
> >$ pantheon-terminal -d ~/Downloads
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-terminal/fix-1208140/+merge/179597
> You are the owner of lp:~jeremywootten/pantheon-terminal/fix-1208140.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-09-16 06:44:53 +0000
+++ CMakeLists.txt	2013-09-22 06:55:21 +0000
@@ -78,3 +78,4 @@
 
 install (TARGETS pantheon-terminal RUNTIME DESTINATION bin)
 install (FILES ${CMAKE_CURRENT_SOURCE_DIR}/data/pantheon-terminal.desktop DESTINATION share/applications)
+install (FILES ${CMAKE_CURRENT_SOURCE_DIR}/data/open-pantheon-terminal-here.desktop DESTINATION share/applications)

=== added file 'data/open-pantheon-terminal-here.desktop'
--- data/open-pantheon-terminal-here.desktop	1970-01-01 00:00:00 +0000
+++ data/open-pantheon-terminal-here.desktop	2013-09-22 06:55:21 +0000
@@ -0,0 +1,11 @@
+[Desktop Entry]
+Name=Open Terminal Here
+TryExec=pantheon-terminal
+Exec=pantheon-terminal -w %u
+Icon=utilities-terminal
+Type=Application
+StartupNotify=true
+X-GNOME-Gettext-Domain=pantheon-terminal
+NoDisplay=True;
+MimeType=inode/directory;x-directory/normal
+

=== modified file 'src/PantheonTerminal.vala'
--- src/PantheonTerminal.vala	2013-06-16 16:31:02 +0000
+++ src/PantheonTerminal.vala	2013-09-22 06:55:21 +0000
@@ -34,7 +34,6 @@
         private static string app_cmd_name;
         private static string app_shell_name;
         public static string? working_directory = null;
-
         /* command_e (-e) is used for running commands independently (not inside a shell) */
         [CCode (array_length = false, array_null_terminated = true)]
         static string[]? command_e = null;
@@ -118,6 +117,17 @@
                 new_window_with_programs (command_e);
                 return 0;
             }
+            if (PantheonTerminalApp.working_directory != null) {
+                uint n_windows = windows.length ();
+                if (n_windows < 1)
+                    new_window_with_working_directory (PantheonTerminalApp.working_directory);
+                else {
+                    unowned PantheonTerminalWindow win = windows.nth_data (n_windows -1);
+                    win.add_tab_with_working_directory (PantheonTerminalApp.working_directory);
+                    win.present ();
+                }
+                return 0;
+            }
             new_window ();
             return 0;
         }
@@ -146,6 +156,14 @@
             return window;
         }
 
+        public PantheonTerminalWindow new_window_with_working_directory (string location) {
+            var window = new PantheonTerminalWindow.with_working_directory (this, location, false);
+            window.show ();
+            windows.append (window);
+            add_window (window);
+            return window;
+        }
+
         public void new_window_with_programs (string[] programs) {
             PantheonTerminalWindow? window;
             window = get_last_window ();
@@ -174,24 +192,32 @@
             { "shell", 's', 0, OptionArg.STRING, ref app_shell_name, N_("Set shell at launch"), "" },
             { "version", 'v', 0, OptionArg.NONE, out print_version, N_("Print version info and exit"), null },
             { "execute" , 'e', 0, OptionArg.STRING_ARRAY, ref command_e, N_("Run a program in terminal"), "" },
-            { "working-directory", 'd', 0, OptionArg.STRING, ref working_directory, N_("Set shell working directory"), "" },
+            { "working-directory", 'w', 0, OptionArg.STRING, ref working_directory, N_("Set shell working directory"), "" },
             { null }
         };
 
         public static int main (string[] args) {
             app_cmd_name = "Pantheon Terminal";
-            
+
             var context = new OptionContext ("Terminal");
             context.add_main_entries (entries, Constants.GETTEXT_PACKAGE);
-            
+
             try {
                 context.parse(ref args);
-            } catch (Error e) {
-                warning (e.message);
+            } catch (Error e) {}
+
+            string[] copy = {};
+            foreach (string s in args)
+                copy += s;
+
+            if (working_directory != null) {
+               /* Recreating -w option so that is passed to instance */
+                copy += "-w";
+                copy += working_directory;
             }
-            
+
             var app = new PantheonTerminalApp ();
-            return app.run (args);
+            return app.run (copy);
         }
     }
 } // Namespace

=== modified file 'src/PantheonTerminalWindow.vala'
--- src/PantheonTerminalWindow.vala	2013-09-19 01:26:44 +0000
+++ src/PantheonTerminalWindow.vala	2013-09-22 06:55:21 +0000
@@ -94,6 +94,18 @@
             init (should_recreate_tabs, false);
         }
 
+        public PantheonTerminalWindow.with_working_directory (Granite.Application app, string location,
+                                                   bool should_recreate_tabs = true) {
+            this.app = app as PantheonTerminalApp;
+            set_application (app);
+            init (should_recreate_tabs, false);
+            new_tab (location);
+        }
+
+        public void add_tab_with_working_directory (string location) {
+            new_tab (location);
+        }
+
         private void init (bool recreate_tabs=true, bool restore_pos = true) {
             this.icon_name = "utilities-terminal";
 

=== modified file 'src/config.vala.cmake'
--- src/config.vala.cmake	2013-03-04 17:26:46 +0000
+++ src/config.vala.cmake	2013-09-22 06:55:21 +0000
@@ -13,4 +13,5 @@
 public const string GENERIC = N_("Terminal");
 public const string NEW_WINDOW = N_("New Window");
 public const string NEW_ROOT_WINDOW = N_("New Root Window");
+public const string OPEN_HERE = N_("Open Terminal Here");
 }