fritzing / fritzing-app

Fritzing desktop application
http://fritzing.org
Other
4.06k stars 838 forks source link

FR: A "reimport part" button #3907

Open JC3 opened 3 years ago

JC3 commented 3 years ago

Problem

There currently seems to be no smooth workflow for updating an imported part from a newer version of an fzpz file. When attempting to import the file, Fritzing will fail and say the part is already loaded (I think it's keyed on moduleId, afaict).

Depending on the situation, sometimes removing the old part from "Mine" then restarting Fritzing works. Sometimes the part needs to be removed from all open projects too. Sometimes it's complicate, sometimes it's easy, but in any case it's weird.

Proposed Solution

A new Parts menu / context menu item (maybe under "Edit Part") that allows you to choose an fzpz file and then reloads the part in the db from that file. This should only be enabled for custom parts, of course.

I'll let somebody else decide what the text of the menu item should be. Something along the lines of "Update Part...", "Reload Part...", "Reimport Part...", etc.

KjellMorgenstern commented 3 years ago

Not feasible, at least not like that. This would corrupt the database and therefore user projects, since there would be no way to tell wether the correct part was loaded for a project. It is intentional that Fritzing rejects those updated files.

It might be inconvenient, but I strongly suggest to manage versions of your parts even when not yet finished, similar to managing source code. We are working on a better preview during development and when editing parts.

JC3 commented 2 years ago

Not feasible, at least not like that. This would corrupt the database and therefore user projects, since there would be no way to tell wether the correct part was loaded for a project. It is intentional that Fritzing rejects those updated files.

Hmm, well, isn't the version of the part used by a project stored in the project's fzz file? How are conflicts resolved if an fzz is loaded that contains a part with the same ID as one in your local parts bin?

Still: So, if I open the parts editor on a custom part that is in use in an open project, make some changes, and then save the part, it pops up that "saving the part will make a change that cannot be undone" prompt, and then everything works fine. I can change every svg for a part and edit all the metadata, and the projects that use it are smoothly updated to show the changes when I exit the parts editor. Can't that same code path be used to reload a part?

It might be inconvenient, but I strongly suggest to manage versions of your parts even when not yet finished, similar to managing source code. We are working on a better preview during development and when editing parts.

I'm not really sure how to do this. When I increment the <version> value in the FZP file, I still receive the "part already exists" error when I attempt to import it. What am I missing?

vanepp commented 2 years ago

This is a bug that I believe relates to alien_files. The fzp file name is being stored in alien_files to be deleted when Fritzing exits if the parts bins are not saved. Here is what is happening. Here (Win 10 Fritzing 0.9.9) C:\Users\owner\Documents\Fritzing\parts\user where loaded fzp files are stored is initially empty.

capture

loading a 74x125 part populates it:

capture1

capture2

now delete the part from the sketch and remove it.

capture3

It looks like the part is gone (and Fritzing thinks so as well, the source shows that it believes it has completely deleted the part! The last comment is "we are about to lose access to the part")

capture4

capture5

However the erase of the fzp file is pending in alien_files until shutdown, so the .fzp file still exists in the C:\Users\owner\Documents\Fritzing\parts\user directory:

capture6

which causes this when I try and reload the same part

capture7

I expect the fix is to remove the fzp file from alien_files (which would require code to scan for the file in alien_files and remove it as alien_files is simply a list of files to delete at shutdown) then delete the file immediately without shutting down Fritzing. If that is done the part should be able to be reloaded successfully. I thought I had already reported this at some point, but I don't see an issue anywhere so perhaps I didn't, and am doing so now.

KjellMorgenstern commented 2 years ago

@JC3 Please see the warning message in @vanepp most recent screenshot above. Don't you see a similar message? It gives you the reason why the part can not be loaded. The scenario which we can not allow to happen:

  1. User A creates a part, ignores the above warning, and publishes it with a duplicated ID
  2. User B loads a part. They have the 'reimport part" button available, so they use it (yes, that would be dump to do, but each of us is prone to not reading warnings and just clicking whatver gets their work done)
  3. User B is happy, new part was imported
  4. Two weeks later: User B discovers some old project of him got destroyed, without any apparent reason. Somewhat rightfully he will be frustrated.

I am not saying that there is absolutely no way for this to already happen with the current version of Fritzing. But it is a lot harder, since both, user A who creates a part, and user B who imports the part, need to jump a lot higher to create this kind of data corruption.

vanepp commented 2 years ago

"@JC3 Please see the warning message in @vanepp most recent screenshot above. Don't you see a similar message? It gives you the reason why the part can not be loaded. "

True, we can't allow duplicate moduleIds in Fritzing (which this avoids) but we can achieve the function he is after (being able to reload a part without having to restart Fritzing) if when the part is removed the code is changed to remove the file from alien_files and deletes the associated files. The part is deleted (and I think, but don't know for sure) that when alien_files is processed on shutdown the files will be deleted anyway because they are no longer in the database, so it may be as easy as just deleting the .fzp and associated .svg files when the part is deleted (we would need to verify adding new files again when reloading the same part, which would cause a duplicate file condition in alien_files doesn't cause any problems though, and deleting the file from alien_files would be an easy way to insure that!) This would be a useful enhancement to Fritzing removing the annoying current work around of having to shut down Fritzing in order to clear an already deleted part.

KjellMorgenstern commented 2 years ago

But why not change the module id instead?

JC3 commented 2 years ago

Will catch up here in a few hours after work. Definitely have some thoughts.

vanepp commented 2 years ago

"But why not change the module id instead?"

Mostly because it is inconvenient, while making a new part, I at least often make mistakes and need to correct them. While I could change the moduleId for each change (or delete the part and shut down Fritzing to clear the part which is what I actually do because it is easier) it would be much more convenient if Fritzing fully deleted the part (including removing it from alien_files) so that I can just reload the part without shutting down Fritzing. I don't think it is a difficult change, it just needs some checking for unexpected side effects (and I don't think there should be any!) Now when you shutdown Fritzing and it processes alien_files to save or delete the added parts, it recognizes that the part has been deleted and deletes the associated files. We would need to check for any condition where that isn't true for some reason, but I can't think of one (which doesn't mean there isn't one though!) What I am suggesting is when the part is deleted, that code should remove the files from the alien_files list (to avoid unforeseen problems) and delete the files immediately (which will add a bit of code for the file deletion.) The effect of that is that now you can reload an updated part without shutting down Fritzing or having to change the moduleId and the file names (since there will be a file name conflict if you maintain the same names on the svg files.) On Fritzing shutdown if the svg files are not also changed, and are still in alien_files they may get deleted because they are associated with the original moduleId that has now changed, when they are still in use for the new part. I expect with the result that the new part won't load due to missing svg files when Fritzing restarts.

KjellMorgenstern commented 1 year ago

@vanepp Thanks for your remarks.

How about

  1. add a "develop" property to a part. Either a property, or a special suffix for the ID. This will enable the following:
  2. Fritzing will just throw away the part (and alien_files) when shut down, no questions asked
  3. Fritzing will show a warning when the part is initially loaded
  4. There will be some kind of reload button or even file change monitor
vanepp commented 1 year ago

We could do that, but I think it would be easier to delete the part in alien_files when the part is deleted in the parts bin. That should be easy to do, we just have to scan alien_files and remove the part files when processing the delete part button. That should solve the problem as the part won't be processed (because it isn't in alien_files) at shutdown without needing any changes to the part. The source says that the part has been fully deleted, but it appears to still be in alien files and thus you can't delete the part (presumably because something knows it is sill in alien_files) without shutting down Fritzing and saving the parts changes to confirm the delete. We would need to figure out how it knows the part still exists (when the source says it has been completely deleted) to make sure this will work though. We may need to make another change if the issue is that the part's existence is in the database somewhere and that is how Fritzing detects that the moduleId is still in use even though it is supposed to have been deleted. To me that seems a cleaner and easier change than adding something to the parts.

vanepp commented 1 year ago

A bit more information on what I mean and the solution I am suggesting. First the problem: import a part

capture1

then remove the part

capture9

capture3 while it says the part is removed it is still in the Fritzing>Parts>user (and svg) directorys

capture5

and I think still registered in the database by the moduleId because removing the files doesn't stop the error message and you get this error message when you try and reload the part without shutting down Fritzing.

capture6

which looks to be triggered by this code segment in src/mainwindow/mainwindow.cpp

capture8

I assume (but don't know for sure) the moduleId from the initial part load is still registered in the database and the files are still in m_alienFiles to be processed (and discarded!) when Fritzing is shut down. What I am suggesting is that we duplicate the code that deletes the reference to the moduleId and removes the part files from m_alienFiles when the part is removed in the original dialog in the source above. This should have the desirable effect of not having to shut down Fritzing before being able to reload the same part again with minimal changes to the code and without a new parameter for parts.

vanepp commented 1 year ago

I just noticed that issue #2985 describes this same issue ...