kicad-developers team mailing list archive

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

Thanks for testing. I had noticed the selection issue on GTK as well, so I
will fix it up before I push it.

The file system watcher is a finiky creature. GTK for sure uses the rename
events (I traced those through the logic last night), but according to the
docs (https://docs.wxwidgets.org/3.1/classwx_file_system_watcher.html) OSX
can't create a rename event, so we get a delete and create event
separately. The selection issue is easy to fix in the rename event, since I
just need to select the new item there. It is slightly more difficult for
OSX though, since we don't always want to have a newly create file be
selected. I can probably work around it though.

I will let you know when I push the change so you can test it.

-Ian

On Sat, 16 Nov 2019, 2:24 p.m. Jeff Young, <jeff@xxxxxxxxx> wrote:

> 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>
> 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
>>>>>>
>>>>>
>>> <0002-Fix-renaming-of-files-in-the-project-tree.patch>
> <0001-Revert-changes-in-the-project-manager-file-tree-for-.patch>
>
>
>
On Sat, 16 Nov 2019, 2:24 p.m. Jeff Young, <jeff@xxxxxxxxx> wrote:

> 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>
> 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
>>>>>>
>>>>>
>>> <0002-Fix-renaming-of-files-in-the-project-tree.patch>
> <0001-Revert-changes-in-the-project-manager-file-tree-for-.patch>
>
>
>