kicad-developers team mailing list archive

[Ian McInerney <Ian.S.McInerney@xxxxxxxx>]

Ok, so I dug into this some more and first of all, the changes that Jeff
pushed still didn't work on GTK.

The root cause of this doesn't appear to be our file system watcher, but
was instead the fact that the tree was being improperly updated. First of
all, we were renaming the old node with the new filename which was causing
the deletion routine in the rename fswatcher handler to never find it and
remove it. Second of all, we were adding the new file into the same
directory as the original file, so it was looking like it was never leaving.

I believe these two patches address the issues, and they work on my GTK box
for both a rename moving directories and a rename inside the same
directory. The first set just reverts the changes that Jeff made to the
code to get it back to the state it was originally, and the second is the
actual new changeset. @Jeff and @Mikolaj, if you could both test these
changes on Windows and OSX to see how they behave that would be good. If
they work for both of you I will push them.

-Ian

On Sat, Nov 16, 2019 at 1:00 AM Mikołaj Wielgus <wielgusmikolaj@xxxxxxxxx>
wrote:

> Jeff,
>
> your new version appears to be working well on my computer on Windows.
>
> Best regards,
> Mikołaj Wielgus
>
>
> On Sat, Nov 16, 2019 at 1:02 AM Jeff Young <jeff@xxxxxxxxx> wrote:
>
>> I pushed a smarter version of my original fix.  @Mikolaj & @Ian if you
>> could test it on Windows and GTK that would be great.
>>
>> Cheers,
>> Jeff.
>>
>>
>> On 15 Nov 2019, at 22:27, Ian McInerney <Ian.S.McInerney@xxxxxxxx> wrote:
>>
>> Scratch my last. It is GTK with the problems. When I rename the file to a
>> new directory with this patch, the tree never seems to update. I have to
>> manually refresh it in order for the file to appear in the correct place.
>>
>> -Ian
>>
>> On Fri, Nov 15, 2019 at 10:21 PM Mikołaj Wielgus <
>> wielgusmikolaj@xxxxxxxxx> wrote:
>>
>>> Yes, I'm on Windows (the details are in the linked related bug report).
>>>
>>> Sorry for the return value problem -- I failed to notice the warnings in
>>> console.
>>>
>>> Best regards,
>>> Mikołaj Wielgus
>>>
>>>
>>> On Fri, Nov 15, 2019 at 11:07 PM Ian McInerney <Ian.S.McInerney@xxxxxxxx>
>>> wrote:
>>>
>>>> I'll give it a test on GTK once my build here finishes, but I don't
>>>> think I have seen any issues with file watcher on GTK in the past.
>>>>
>>>> -Ian
>>>>
>>>> On Fri, Nov 15, 2019 at 10:03 PM Jeff Young <jeff@xxxxxxxxx> wrote:
>>>>
>>>>> Hi Mikolaj,
>>>>>
>>>>> The Mac compiler doesn’t like Rename() returning values when the
>>>>> return type is void.  However, after fixing that it works fine on Mac.
>>>>>
>>>>> I remember however something about the file watcher not working on all
>>>>> platforms.  I thought the problem platform was Windows, though, so maybe
>>>>> I’m not remembering it correctly (as you’re on Windows, right?).
>>>>>
>>>>> Can someone validate that this works on GTK?
>>>>>
>>>>> Cheers,
>>>>> Jeff.
>>>>>
>>>>>
>>>>> On 15 Nov 2019, at 21:05, Mikołaj Wielgus <wielgusmikolaj@xxxxxxxxx>
>>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Renaming file to a different directory causes the entire project tree
>>>>> to be recreated, which causes all subdirectories to collapse, unexpectedly
>>>>> to the user.
>>>>>
>>>>> This patch solves the problem by deleting the original node after
>>>>> moving the file. Then the file watcher raises an event whose handler
>>>>> constructs a new node in the new location.
>>>>>
>>>>> This issue comes from the fix to this bug:
>>>>> https://bugs.launchpad.net/kicad/+bug/1852431
>>>>>
>>>>> Best Regards,
>>>>> Mikołaj Wielgus
>>>>> <0001-Do-not-reconstruct-proj-tree-on-rename-to-diff-dir.patch>
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>
>>>>
>>

From 6255dcfdbda8efc87d9c15e41f12adc4908f0c64 Mon Sep 17 00:00:00 2001
From: Ian McInerney <Ian.S.McInerney@xxxxxxxx>
Date: Sat, 16 Nov 2019 01:35:48 +0000
Subject: [PATCH 2/2] Fix renaming of files in the project tree

We must add the new node for the new file in the new directory,
not in the original root directory. The file watcher will also
delete the old node, so there is no point in renaming the file.

Fixes: lp:1852357
* https://bugs.launchpad.net/kicad/+bug/1852357
---
 kicad/tree_project_frame.cpp | 4 +++-
 kicad/treeproject_item.cpp   | 2 --
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kicad/tree_project_frame.cpp b/kicad/tree_project_frame.cpp
index 6ec5af712..4f4a17075 100644
--- a/kicad/tree_project_frame.cpp
+++ b/kicad/tree_project_frame.cpp
@@ -897,6 +897,7 @@ void TREE_PROJECT_FRAME::OnFileSystemEvent( wxFileSystemWatcherEvent& event )
     case wxFSW_EVENT_RENAME :
         {
             const wxFileName& newpath = event.GetNewPath();
+            wxString newdir = newpath.GetPath();
             wxString newfn = newpath.GetFullPath();
 
             while( kid.IsOk() )
@@ -912,7 +913,8 @@ void TREE_PROJECT_FRAME::OnFileSystemEvent( wxFileSystemWatcherEvent& event )
                 kid = m_TreeProject->GetNextChild( root_id, cookie );
             }
 
-            AddItemToTreeProject( newfn, root_id, false );
+            wxTreeItemId newroot_id = findSubdirTreeItem( newdir );
+            AddItemToTreeProject( newfn, newroot_id, false );
         }
         break;
     }
diff --git a/kicad/treeproject_item.cpp b/kicad/treeproject_item.cpp
index c994912ac..e54ee0947 100644
--- a/kicad/treeproject_item.cpp
+++ b/kicad/treeproject_item.cpp
@@ -120,8 +120,6 @@ bool TREEPROJECT_ITEM::Rename( const wxString& name, bool check )
         return false;
     }
 
-    SetFileName( newFile );
-
     return true;
 }
 
-- 
2.21.0

From 22fc6e47eb9bd98200cd18aedbdef55408ac21b8 Mon Sep 17 00:00:00 2001
From: Ian McInerney <Ian.S.McInerney@xxxxxxxx>
Date: Sat, 16 Nov 2019 01:46:40 +0000
Subject: [PATCH 1/2] Revert changes in the project manager file tree for
 rename handling

Revert "Be a bit smarter about moving files through a rename."
Revert "Check for file moving directory and refresh entire tree if so."

This reverts commit f8aea249df9d36e120f8c54da7e14154cb1f7c2a.
This reverts commit 8ce04d3362518cf1a54e87cb8d5bf3bd183955ed.
---
 kicad/tree_project_frame.cpp | 43 ++++++++----------------------------
 kicad/tree_project_frame.h   |  4 ++--
 2 files changed, 11 insertions(+), 36 deletions(-)

diff --git a/kicad/tree_project_frame.cpp b/kicad/tree_project_frame.cpp
index 458429f6f..6ec5af712 100644
--- a/kicad/tree_project_frame.cpp
+++ b/kicad/tree_project_frame.cpp
@@ -292,8 +292,8 @@ wxString TREE_PROJECT_FRAME::GetFileExt( TreeFileType type )
 }
 
 
-wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wxTreeItemId& aRoot,
-                                                       bool aRecurse )
+bool TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName,
+                                               wxTreeItemId& aRoot, bool aRecurse )
 {
     wxTreeItemId    cellule;
     TreeFileType    type = TREE_UNKNOWN;
@@ -302,7 +302,7 @@ wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wx
     // Files/dirs names starting by "." are not visible files under unices.
     // Skip them also under Windows
     if( fn.GetName().StartsWith( wxT( "." ) ) )
-        return 0;
+        return false;
 
     if( wxDirExists( aName ) )
     {
@@ -333,7 +333,7 @@ wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wx
         }
 
         if( !addFile )
-            return 0;
+            return false;
 
         // only show the schematic if it is a top level schematic.  Eeschema
         // cannot open a schematic and display it properly unless it starts
@@ -365,7 +365,7 @@ wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wx
                 fp = wxFopen( fullFileName, wxT( "rt" ) );
 
                 if( fp == NULL )
-                    return 0;
+                    return false;
 
                 addFile = false;
 
@@ -385,7 +385,7 @@ wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wx
                 fclose( fp );
 
                 if( !addFile )
-                    return 0; // it is a non-top-level schematic
+                    return false; // it is a non-top-level schematic
             }
         }
 
@@ -418,7 +418,7 @@ wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wx
         if( itemData )
         {
             if( itemData->GetFileName() == aName )
-                return kid;
+                return true;    // well, we would have added it, but it is already here!
         }
 
         kid = m_TreeProject->GetNextChild( aRoot, cookie );
@@ -467,7 +467,7 @@ wxTreeItemId TREE_PROJECT_FRAME::AddItemToTreeProject( const wxString& aName, wx
         m_TreeProject->SortChildren( cellule );
     }
 
-    return data->GetId();
+    return true;
 }
 
 
@@ -699,33 +699,8 @@ void TREE_PROJECT_FRAME::OnRenameFile( wxCommandEvent& )
     if( buffer.IsEmpty() )
         return; // empty file name not allowed
 
-    wxString oldDir = tree_data->GetDir();
-
     if( tree_data->Rename( buffer, true ) )
-    {
-        if( tree_data->GetDir() != oldDir )
-        {
-            wxFileName filename( tree_data->GetFileName() );
-            filename.Normalize();
-
-            wxTreeItemId parent = findSubdirTreeItem( filename.GetPath() );
-            m_TreeProject->SelectItem( tree_data->GetId(), false );
-            m_TreeProject->Delete( tree_data->GetId() );
-
-            wxTreeItemId new_item = AddItemToTreeProject( filename.GetFullPath(), parent, false );
-
-            if( new_item )
-            {
-                m_TreeProject->SortChildren( parent );
-                m_TreeProject->ScrollTo( new_item );
-                m_TreeProject->SelectItem( new_item, true );
-            }
-        }
-        else
-        {
-            m_TreeProject->SetItemText( curr_item, buffer );
-        }
-    }
+        m_TreeProject->SetItemText( curr_item, buffer );
 }
 
 
diff --git a/kicad/tree_project_frame.h b/kicad/tree_project_frame.h
index 57ab6cc45..80cae2927 100644
--- a/kicad/tree_project_frame.h
+++ b/kicad/tree_project_frame.h
@@ -158,9 +158,9 @@ private:
      * @param aRoot = the wxTreeItemId item where to add sub tree items
      * @param aRecurse = true to add file or subdir names to the current tree item
      *                   false to stop file add.
+     * @return true if the file (or directory) is added.
      */
-    wxTreeItemId AddItemToTreeProject( const wxString& aName, wxTreeItemId& aRoot,
-                                       bool aRecurse = true );
+    bool AddItemToTreeProject( const wxString& aName, wxTreeItemId& aRoot, bool aRecurse = true );
 
     /**
      * Function findSubdirTreeItem
-- 
2.21.0