← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Kicad: project manager: Do not reconstruct entire project tree when renaming file to different dir

 

How bizarre.  Make you wonder how it worked on Mac and Windows.  (Hmmm… I wonder if GTK goes through wxFSW_EVENT_RENAME and Mac and Windows go through wxFSW_EVENT_DELETE and wxFSW_EVENT_CREATE?)

Your patch works on OSX except for one thing: it doesn’t get the selection right (selecting the next item in the moved-from location rather than keeping the moved item selected).

Cheers,
Jeff.


> On 16 Nov 2019, at 01:57, Ian McInerney <Ian.S.McInerney@xxxxxxxx> wrote:
> 
> 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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <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 <https://launchpad.net/~kicad-developers>
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>>> More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
>> 
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>> More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
> 
> <0002-Fix-renaming-of-files-in-the-project-tree.patch><0001-Revert-changes-in-the-project-manager-file-tree-for-.patch>


Follow ups

References