Skip to content

Commit

Permalink
Fix tab label disambiguation regression (#2081)
Browse files Browse the repository at this point in the history 
Co-authored-by: Ryan Kornheisl <ryan@skarva.tech>
  • Loading branch information
@zeebok
Jeremy Wootten and zeebok authored Mar 6, 2023
1 parent c4d8cb6 commit 75e57b0
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 75 deletions.
76 changes: 76 additions & 0 deletions libcore/FileUtils.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1234,6 +1234,82 @@ namespace Files.FileUtils {
return false;
}
}

// Return enough of @path to distinguish it from @conflict_path
// Currently, differences in some parts of uri are ignored, only scheme and path are used.
public string disambiguate_uri (string uri, string conflict_uri) {
string? uri_scheme, uri_userinfo, uri_host, uri_path, uri_query, uri_fragment;
string? conflict_scheme, conflict_userinfo, conflict_host, conflict_path, conflict_query, conflict_fragment;
int uri_port, conflict_port;
try {
Uri.split (uri, UriFlags.NONE, out uri_scheme, out uri_userinfo, out uri_host, out uri_port, out uri_path, out uri_query, out uri_fragment);
Uri.split (conflict_uri, UriFlags.NONE, out conflict_scheme, out conflict_userinfo, out conflict_host, out conflict_port, out conflict_path, out conflict_query, out conflict_fragment);
} catch {
return Path.get_basename (uri);
}

var prefix = "";
var conflict_prefix = "";
var temp_path = uri_path;
var temp_conflict_path = conflict_path;
string temp_basename = "", temp_conflict_basename = "";
var basename = Path.get_basename (temp_path);

if (basename == "") {
return (uri_scheme ?? "file") + "://";
}

// This function should be called with an actually conflicting path but
// we deal with some unexpected values in case.
if (basename != Path.get_basename (conflict_path)) {
return basename;
}

// Deal with same path possibly with different schemes
if (temp_path == temp_conflict_path) {
if ((uri_scheme ?? "file") == (conflict_scheme ?? "file")) {
return basename;
} else {
return uri;
}
}

// Add parent directories until path and conflict path differ
// Protect from possible infinite loops
uint count = 0;
while (prefix == conflict_prefix &&
temp_basename != Path.DIR_SEPARATOR_S &&
count < 10
) {
var parent_temp_path = FileUtils.get_parent_path_from_path (temp_path, false);
var parent_temp_confict_path = FileUtils.get_parent_path_from_path (temp_conflict_path, false);
temp_path = parent_temp_path;
temp_conflict_path = parent_temp_confict_path;
temp_basename = Path.get_basename (parent_temp_path);
temp_conflict_basename = Path.get_basename (parent_temp_confict_path);

if (temp_basename != Path.DIR_SEPARATOR_S) {
prefix = temp_basename + Path.DIR_SEPARATOR_S + prefix;
} else {
prefix = temp_basename + prefix;
}

if (temp_conflict_basename != Path.DIR_SEPARATOR_S) {
conflict_prefix = temp_conflict_basename + Path.DIR_SEPARATOR_S + conflict_prefix;
} else {
conflict_prefix = temp_conflict_basename + conflict_prefix;
}

count++;
}

if (count > 5) {
warning ("disambiguate_oath: too many loops");
return basename;
}

return (prefix + basename);
}
}

namespace Files {
Expand Down
58 changes: 58 additions & 0 deletions libcore/tests/UtilTests/UtilTests.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,64 @@ void add_file_utils_tests () {
assert (!result.contains (Files.FileUtils.COPY_TAG));
assert (result.contains ("2"));
});

Test.add_func ("/FileUtils/disambiguate_path/conflicting", () => {
var result = Files.FileUtils.disambiguate_uri (
"/A/B/folder",
"/A/C/folder"
);
assert (result == "B/folder");
});

Test.add_func ("/FileUtils/disambiguate_path/different_length", () => {
var result = Files.FileUtils.disambiguate_uri (
"/A/B/folder",
"X/A/B/folder"
);
assert (result == "/A/B/folder");

result = Files.FileUtils.disambiguate_uri (
"X/A/B/folder",
"/A/B/folder"
);
assert (result == "X/A/B/folder");
});

Test.add_func ("/FileUtils/disambiguate_path/not_conflicting", () => {
var result = Files.FileUtils.disambiguate_uri (
"/A/B/folder",
"/A/B/folder2"
);
assert (result == "folder");
});

Test.add_func ("/FileUtils/disambiguate_path/equivalent", () => {
var result = Files.FileUtils.disambiguate_uri (
"/A/B/folder",
"file:///A/B/folder"
);
assert (result == "folder");

result = Files.FileUtils.disambiguate_uri (
"file:///A/B/folder",
"/A/B/folder"
);
assert (result == "folder");
});

Test.add_func ("/FileUtils/disambiguate_path/different_scheme", () => {
var result = Files.FileUtils.disambiguate_uri (
"/A/B/folder",
"smb:///A/B/folder"
);
assert (result == "/A/B/folder");

result = Files.FileUtils.disambiguate_uri (
"smb:///A/B/folder",
"/A/B/folder"
);
assert (result == "smb:///A/B/folder");
});
}

int main (string[] args) {
Expand Down
24 changes: 5 additions & 19 deletions src/View/ViewContainer.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,6 @@

namespace Files.View {
public class ViewContainer : Gtk.Box {
private static int container_id;

protected static int get_next_container_id () {
return ++container_id;
}

static construct {
container_id = -1;
}

public int id {get; construct;}
public Gtk.Widget? content_item;
public bool can_show_folder { get; private set; default = false; }
private View.Window? _window = null;
Expand Down Expand Up @@ -113,11 +102,6 @@ namespace Files.View {
public signal void tab_name_changed (string tab_name);
public signal void loading (bool is_loading);
public signal void active ();
/* path-changed signal no longer used */

construct {
id = ViewContainer.get_next_container_id ();
}

/* Initial location now set by Window.make_tab after connecting signals */
public ViewContainer (View.Window win) {
Expand Down Expand Up @@ -189,6 +173,8 @@ namespace Files.View {
}
}

// Either the path or a special name or fallback if invalid
// Window will use as little as possible to distinguish tabs
private string label = "";
public string tab_name {
private set {
Expand Down Expand Up @@ -358,16 +344,16 @@ namespace Files.View {
}

private void update_tab_name () {
string tab_name = Files.INVALID_TAB_NAME;
var tab_name = Files.INVALID_TAB_NAME;

string protocol, path;
FileUtils.split_protocol_from_path (this.uri, out protocol, out path); //path is escaped
FileUtils.split_protocol_from_path (this.uri, out protocol, out path);
if (path == "" || path == Path.DIR_SEPARATOR_S) {
tab_name = Files.protocol_to_name (protocol);
} else if (protocol == "" && path == Environment.get_home_dir ()) {
tab_name = _("Home");
} else {
tab_name = Path.get_basename (Uri.unescape_string (path));
tab_name = Uri.unescape_string (path);
}

this.tab_name = tab_name;
Expand Down
107 changes: 51 additions & 56 deletions src/View/Window.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,11 +503,9 @@ namespace Files.View {

change_tab ((int)tabs.insert_tab (tab, -1));
tabs.current = tab;
/* Capturing ViewContainer object reference in closure prevents its proper destruction
* so capture its unique id instead */
var id = content.id;

content.tab_name_changed.connect ((tab_name) => {
set_tab_label (check_for_tab_with_same_name (id, tab_name), tab, tab_name);
check_for_tabs_with_same_name (); // Also sets tab_label.
});

content.loading.connect ((is_loading) => {
Expand Down Expand Up @@ -566,47 +564,64 @@ namespace Files.View {
return -1;
}

private string check_for_tab_with_same_name (int id, string path) {
if (path == Files.INVALID_TAB_NAME) {
return path;
}

var new_label = Path.get_basename (path);
foreach (Granite.Widgets.Tab tab in tabs.tabs) {
/** Compare every tab label with every other and resolve ambiguities **/
private void check_for_tabs_with_same_name () {
// Take list copy so foreach clauses can be nested safely
var copy_tabs = tabs.tabs.copy ();
foreach (unowned var tab in tabs.tabs) {
var content = (ViewContainer)(tab.page);
if (content.id != id) {
string content_path = content.tab_name;
string content_label = Path.get_basename (content_path);
if (tab.label == new_label) {
if (content_path != path) {
new_label = disambiguate_name (new_label, path, content_path); /*Relabel calling tab */
if (content_label == tab.label) {
/* Also relabel conflicting tab (but not before this function finishes) */
Idle.add_full (GLib.Priority.LOW, () => {
var unique_name = disambiguate_name (content_label, content_path, path);
set_tab_label (unique_name, tab, content_path);
return GLib.Source.REMOVE;
});
}
}
} else if (content_label == new_label &&
content_path == path &&
content_label != tab.label) {

/* Revert to short label when possible */
Idle.add_full (GLib.Priority.LOW, () => {
set_tab_label (content_label, tab, content_path);
return GLib.Source.REMOVE;
});
if (content.tab_name == Files.INVALID_TAB_NAME) {
set_tab_label (content.tab_name, tab, content.tab_name);
continue;
}

var path = content.location.get_path ();
if (path == null) { // e.g. for uris like network://
set_tab_label (content.tab_name, tab, content.tab_name);
continue;
}
var basename = Path.get_basename (path);

// Ignore content not named from the path
if (!content.tab_name.has_suffix (basename)) {
set_tab_label (content.tab_name, tab, content.tab_name);
continue;
}

// Tab label defaults to the basename.
set_tab_label (basename, tab, content.tab_name);

// Compare with every other tab for same label
foreach (unowned var tab2 in copy_tabs) {
var content2 = (ViewContainer)(tab2.page);
if (content2 == content || content2.tab_name == Files.INVALID_TAB_NAME) {
continue;
}

var path2 = content2.location.get_path ();
if (path2 == null) { // e.g. for uris like network://
continue;
}
var basename2 = Path.get_basename (path2);

// Ignore content not named from the path
if (!content2.tab_name.has_suffix (basename2)) {
continue;
}

if (basename2 == basename && path2 != path) {
set_tab_label (FileUtils.disambiguate_uri (path2, path), tab2, content2.tab_name);
set_tab_label (FileUtils.disambiguate_uri (path, path2), tab, content.tab_name);
}
}
}

return new_label;
return;
}

/* Just to append "as Administrator" when appropriate */
private void set_tab_label (string label, Granite.Widgets.Tab tab, string? tooltip = null) {

string lab = label;
if (Files.is_admin ()) {
lab += (" " + _("(as Administrator)"));
Expand All @@ -626,25 +641,6 @@ namespace Files.View {
}
}

private string disambiguate_name (string name, string path, string conflict_path) {
string prefix = "";
string prefix_conflict = "";
string path_temp = path;
string conflict_path_temp = conflict_path;

/* Add parent directories until path and conflict path differ */
while (prefix == prefix_conflict) {
var parent_path= FileUtils.get_parent_path_from_path (path_temp);
var parent_conflict_path = FileUtils.get_parent_path_from_path (conflict_path_temp);
prefix = Path.get_basename (parent_path) + Path.DIR_SEPARATOR_S + prefix;
prefix_conflict = Path.get_basename (parent_conflict_path) + Path.DIR_SEPARATOR_S + prefix_conflict;
path_temp= parent_path;
conflict_path_temp = parent_conflict_path;
}

return prefix + name;
}

public void bookmark_uri (string uri, string custom_name = "") {
sidebar.add_favorite_uri (uri, custom_name);
}
Expand All @@ -653,7 +649,6 @@ namespace Files.View {
return !sidebar.has_favorite_uri (uri);
}


public void remove_content (ViewContainer view_container) {
remove_tab (tabs.get_tab_by_widget ((Gtk.Widget)view_container));
}
Expand Down

0 comments on commit 75e57b0

Please sign in to comment.