Merge lp:~gero-bare/scratch/fix-bug-1633916 into lp:~elementary-apps/scratch/scratch

Proposed by Gero.Bare
Status: Rejected
Rejected by: Adam Bieńkowski
Proposed branch: lp:~gero-bare/scratch/fix-bug-1633916
Merge into: lp:~elementary-apps/scratch/scratch
Diff against target: 116 lines (+67/-2)
2 files modified
plugins/filemanager/FileManagerPlugin.vala (+33/-2)
plugins/filemanager/FileView.vala (+34/-0)
To merge this branch: bzr merge lp:~gero-bare/scratch/fix-bug-1633916
Reviewer Review Type Date Requested Status
Zisu Andrei (community) Needs Fixing
Review via email: mp+309190@code.launchpad.net

Description of the change

This might be an overkill but I'm not aware of a better solution.

On doc closes check if the document is currently selected and de select it from the file view.

I keep track of the documents in order to disconnect the signals when opportune.

To post a comment you must log in.
1772. By Gero.Bare

Fix formatting error.

Revision history for this message
Zisu Andrei (matzipan) wrote :

I confirm your branch fixes the issue and does not introduce any regressions.

However, there's a tiny comment inline.

review: Needs Fixing
Revision history for this message
Zisu Andrei (matzipan) wrote :

Actually, I think I spotted a bug: what happens if the user has 2 tabs open for the same file (right click on the tab and click "duplicate"). When one of them is closed, the item is deselected and no deselection occurs for the second one.

review: Needs Fixing
1773. By Gero.Bare

Now theflemanger selects atomatically the document currently opened in the editor.
Fixed some style issues.

1774. By Gero.Bare

Fix a little screw up. It should compile now.

Revision history for this message
Zisu Andrei (matzipan) wrote :

Hey Gero,

Great progress, but I found a little bug.

How to test:
Open a file (say file a)
Then click the settings icon in the top right and click "Add View".
Now press ctrl+o and open a different file (say file b)
Then press ctrl+o and open the same file as the one you opened in your first step (say file a).

You will have two views, one with two tabs, a and b, and one with a single tab, a.

In the view with two tabs. Close "a". This will leave you with a view with "a", and one with "b".

Press the "a" tab. Your selection won't happen.

Also, there's some comments inline.

Let's get this merged!

review: Needs Fixing
1775. By Gero.Bare

Don't reopen documents unnecessarily.

Revision history for this message
Gero.Bare (gero-bare) wrote :

document.focus () => doesn't focus a shit. Sorry my language but there's only so much I can do it here.

Besides that, I think this works.

Revision history for this message
Zisu Andrei (matzipan) wrote :

Can anybody reject this branch? I'm working on an improvement here: https://code.launchpad.net/~matzipan/scratch/select-file-in-sidebar/+merge/313838

Unmerged revisions

1775. By Gero.Bare

Don't reopen documents unnecessarily.

1774. By Gero.Bare

Fix a little screw up. It should compile now.

1773. By Gero.Bare

Now theflemanger selects atomatically the document currently opened in the editor.
Fixed some style issues.

1772. By Gero.Bare

Fix formatting error.

1771. By Gero.Bare

On document close, check if the file is currently selected and deselect it.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/filemanager/FileManagerPlugin.vala'
--- plugins/filemanager/FileManagerPlugin.vala 2016-09-03 11:50:58 +0000
+++ plugins/filemanager/FileManagerPlugin.vala 2016-11-18 21:54:58 +0000
@@ -27,6 +27,7 @@
27 27
28 Gtk.Box box;28 Gtk.Box box;
29 FileManager.FileView view;29 FileManager.FileView view;
30 private Gee.TreeSet <Scratch.Services.Document> docs;
3031
31 public Object object { owned get; construct; }32 public Object object { owned get; construct; }
3233
@@ -37,11 +38,21 @@
37 public void activate () {38 public void activate () {
38 plugins = (Scratch.Services.Interface) object;39 plugins = (Scratch.Services.Interface) object;
39 plugins.hook_notebook_sidebar.connect (on_hook_sidebar);40 plugins.hook_notebook_sidebar.connect (on_hook_sidebar);
41
42 docs = new Gee.TreeSet <Scratch.Services.Document> ();
43 plugins.hook_document.connect (on_hook_document);
40 }44 }
4145
42 public void deactivate () {46 public void deactivate () {
43 if (box != null)47 if (box != null)
44 box.destroy();48 box.destroy();
49
50 foreach (var doc in docs) {
51 docs.remove (doc);
52 doc.doc_closed.disconnect (on_doc_closed);
53 }
54
55 plugins.hook_document.disconnect (on_hook_document);
45 }56 }
4657
47 public void update_state () {58 public void update_state () {
@@ -57,8 +68,14 @@
57 // File View68 // File View
58 view = new FileManager.FileView ();69 view = new FileManager.FileView ();
5970
60 view.select.connect ((a) => {71 view.select.connect ((path) => {
61 var file = GLib.File.new_for_path (a);72 foreach (var document in docs) {
73 if (document.file.get_path () == path) {
74 document.focus ();
75 return;
76 }
77 }
78 var file = GLib.File.new_for_path (path);
62 plugins.open_file (file);79 plugins.open_file (file);
63 });80 });
6481
@@ -103,6 +120,20 @@
103120
104 notebook.append_page (box, new Gtk.Label (_("File Manager")));121 notebook.append_page (box, new Gtk.Label (_("File Manager")));
105 }122 }
123
124 void on_hook_document (Scratch.Services.Document doc) {
125 if (!docs.contains (doc)) {
126 docs.add (doc);
127 doc.doc_closed.connect (on_doc_closed);
128 }
129
130 view.select_item_from_path (doc.file.get_path ());
131 }
132
133 void on_doc_closed (Scratch.Services.Document doc) {
134 docs.remove (doc);
135 view.deselect_file_path (doc.file.get_path ());
136 }
106 }137 }
107}138}
108139
109140
=== modified file 'plugins/filemanager/FileView.vala'
--- plugins/filemanager/FileView.vala 2016-09-03 11:50:58 +0000
+++ plugins/filemanager/FileView.vala 2016-11-18 21:54:58 +0000
@@ -102,6 +102,40 @@
102 this.selected = null;102 this.selected = null;
103 }103 }
104104
105 public void deselect_file_path (string path) {
106 if (selected != null) {
107 if ((selected as FileItem).file.file.get_path () == path) {
108 selected = null;
109 }
110 }
111 }
112
113 public void select_item_from_path (string path) {
114 debug ("Select filepath %s", path);
115 if (selected == null || (selected as FileItem).path != path) {
116 var file = new File (path);
117 if (file.is_valid_directory) {
118 return;
119 }
120
121 var parent_folder = new File(file.file.get_parent ().get_path ());
122 open_folder (parent_folder);
123
124 if (this.folder == null) {
125 debug ("Failed to open %s", parent_folder.file.get_path ());
126 return;
127 }
128
129 foreach (var item in this.folder.children) {
130 if ((item as Item).path == path) {
131 selected = (item as Item);
132 break;
133 }
134 }
135 debug ("Failed to open filepath %s", path);
136 }
137 }
138
105 private bool is_open (File folder) {139 private bool is_open (File folder) {
106 foreach (var child in root.children) {140 foreach (var child in root.children) {
107 if (folder.path == (child as Item).path) {141 if (folder.path == (child as Item).path) {

Subscribers

People subscribed via source and target branches