jucipp icon indicating copy to clipboard operation
jucipp copied to clipboard

notebook tabs shows first uncommon subdirectory if filename is the same

Open zalox opened this issue 7 years ago • 4 comments

zalox avatar Feb 11 '18 16:02 zalox

I had a look at this, and there were some minor issues:

  • The labels got the /.../-prefix even though only the filenames were equal
  • When closing a tab with a path prefix, the tabs did not get updated.
  • update_tab_label should be called when opening a new buffer, just in case the buffer is empty and on_change is never called.
  • Maybe slightly more complex code than needed.

I did a fix proposal here that you can look at:

diff --git a/src/notebook.cc b/src/notebook.cc
index 0cbdf75..4f4c654 100644
--- a/src/notebook.cc
+++ b/src/notebook.cc
@@ -10,14 +10,6 @@
 #include "source_language_protocol.h"
 #include "gtksourceview-3.0/gtksourceview/gtksourcemap.h"
 
-template <class firstIt, class secondIt>
-  auto mismatch(firstIt first1, firstIt last1, secondIt first2,secondIt last2){
-  while (first1 != last1 && first2 != last2 && *first1 == *first2) {
-      ++first1, ++first2;
-  }
-  return std::make_pair(*first1, *first2);
-};
-
 Notebook::TabLabel::TabLabel(const boost::filesystem::path &path, std::function<void()> on_close) {
   set_can_focus(false);
   set_tooltip_text(path.string());
@@ -26,7 +18,6 @@ Notebook::TabLabel::TabLabel(const boost::filesystem::path &path, std::function<
   auto hbox=Gtk::manage(new Gtk::Box());
   
   hbox->set_can_focus(false);
-  label.set_text(path.filename().string()+' ');
   label.set_can_focus(false);
   button->set_image_from_icon_name("window-close-symbolic", Gtk::ICON_SIZE_MENU);
   button->set_can_focus(false);
@@ -203,12 +194,10 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
     }
   };
   source_views.back()->update_tab_label=[this](Source::View *view) {
-    const auto update_label = [&](const boost::filesystem::path &path, size_t index){
+    const auto update_label = [this](size_t index, const std::string &prepend) {
       const auto current_view = source_views[index];
-      std::string title = current_view->file_path.filename().string();
-      if(!path.empty())
-        title = path.string() + "/.../" + title;
-      if(view->get_buffer()->get_modified())
+      auto title = prepend+current_view->file_path.filename().string();
+      if(current_view->get_buffer()->get_modified())
         title+='*';
       else
         title+=' ';
@@ -216,32 +205,32 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
       tab_label->label.set_text(title);
       tab_label->set_tooltip_text(filesystem::get_short_path(current_view->file_path).string());
     };
-    std::unordered_map<int, std::pair<boost::filesystem::path, boost::filesystem::path>> uncommon_paths;
     const auto file_name=view->file_path.filename();
     size_t current_view_index = 0;
+    std::string prepend_current_view;
     for(size_t c=0;c<size();++c) {
       if(source_views[c]==view) {
         current_view_index = c;
         continue;
       }
       if (source_views[c]->file_path.filename() == file_name) {
-        auto const first_uncommon_path = mismatch(
-          view->file_path.parent_path().rbegin(),
-          view->file_path.parent_path().rend(),
-          source_views[c]->file_path.parent_path().rbegin(),
-          source_views[c]->file_path.parent_path().rend()
-        );
-        uncommon_paths.emplace(std::make_pair(c, first_uncommon_path));
+        int diff=0;
+        auto parent_path1=view->file_path.parent_path();
+        auto parent_path2=source_views[c]->file_path.parent_path();
+        auto it1=parent_path1.rbegin();
+        auto it2=parent_path2.rbegin();
+        while (it1 != parent_path1.rend() &&
+               it2 != parent_path2.rend() && *it1 == *it2) {
+          ++it1;
+          ++it2;
+          ++diff;
+        }
+        if(prepend_current_view.empty())
+          prepend_current_view=it1->string()+(diff>0?"/.../":"/");
+        update_label(c, it2->string()+(diff>0?"/.../":"/"));
       }
     }
-    for (const auto &uncommon_path : uncommon_paths) {
-      update_label(uncommon_path.second.second, uncommon_path.first);
-    }
-    if (uncommon_paths.empty()) {
-      update_label("", current_view_index);
-    } else {
-      update_label(uncommon_paths.begin()->second.first, current_view_index);
-    }
+    update_label(current_view_index, prepend_current_view);
     update_status(view);
   };
   source_views.back()->update_status_diagnostics=[this](Source::View* view) {
@@ -327,6 +316,8 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
     if(index!=static_cast<size_t>(-1))
       close(index);
   }));
+  if(source_views.back()->update_tab_label)
+    source_views.back()->update_tab_label(source_views.back());
   
   //Add star on tab label when the page is not saved:
   source_view->get_buffer()->signal_modified_changed().connect([this, source_view]() {
@@ -540,6 +531,11 @@ bool Notebook::close(size_t index) {
     hboxes.erase(hboxes.begin()+index);
     tab_labels.erase(tab_labels.begin()+index);
   }
+  
+  for(auto view: get_views()) { // Update all view tabs in case one clicks cross to close a buffer
+    if(view->update_tab_label)
+      view->update_tab_label(view);
+  }
   return true;
 }

eidheim avatar Feb 12 '18 09:02 eidheim

Here are some suggestion for changes that also fixes double / when root path is shown:

diff --git a/src/notebook.cc b/src/notebook.cc
index c4dd4c1..4d7c5ef 100644
--- a/src/notebook.cc
+++ b/src/notebook.cc
@@ -209,6 +209,7 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
     const auto file_name = view->file_path.filename();
     std::string prepend_current_view;
     size_t current_view_index = 0;
+    // Extends tab label with a parent folder if two tab labels show the same filename
     for (size_t c = 0; c < size(); ++c) {
       if (source_views[c] == view) {
         current_view_index = c;
@@ -218,16 +219,26 @@ void Notebook::open(const boost::filesystem::path &file_path_, size_t notebook_i
         int diff = 0;
         const auto parent_path1 = view->file_path.parent_path();
         const auto parent_path2 = source_views[c]->file_path.parent_path();
-        auto it1 = parent_path1.rbegin();
-        auto it2 = parent_path2.rbegin();
-        while (it1 != parent_path1.rend() && it2 != parent_path2.rend() && *it1 == *it2) {
+        // Ubuntu 16.04 workaround:
+        auto it1 = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path1.end());
+        auto it1_end = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path1.begin());
+        auto it2 = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path2.end());
+        auto it2_end = std::reverse_iterator<boost::filesystem::path::iterator>(parent_path2.begin());
+        while (it1 != it1_end && it2 != it2_end && *it1 == *it2) {
           ++it1;
           ++it2;
           ++diff;
         }
-        if (prepend_current_view.empty())
-          prepend_current_view = it1->string() + (diff > 0 ? "/.../" : "/");
-        update_label(c, it2->string() + (diff > 0 ? "/.../" : "/"));
+        if (prepend_current_view.empty()) {
+          auto it1_str = it1->string();
+          if(it1_str=="/") // Fix for root directory
+            it1_str="";
+          prepend_current_view = it1_str + (diff > 0 ? "/.../" : "/");
+        }
+        auto it2_str = it2->string();
+        if(it2_str=="/") // Fix for root directory
+          it2_str="";
+        update_label(c, it2_str + (diff > 0 ? "/.../" : "/"));
       }
     }
     update_label(current_view_index, prepend_current_view);

eidheim avatar Feb 24 '18 09:02 eidheim

One problem though is that if you have a directory structure like this:

  • a/
    • b/
      • 1.txt
    • 1.txt
  • b/
    • 1.txt

and you open all the 1.txt files, then if you modify them, their labels will change depending on which file you modify.

eidheim avatar Feb 24 '18 09:02 eidheim

I will have time to look at this this today or this weekend. Feel free to fix it.

zalox avatar Feb 28 '18 12:02 zalox