New take on project export: now a zip file is generated which can be imported also

[BFH-ktt1]

No description provided.

[MarcinOrlowski]

If you have ZIP supported now, then I see no point offering the user to expor the folder structure. That's confusing. If user would like to peek, s/he can unzip the file.

[BFH-ktt1]

@MarcinOrlowski : I do not offer the user to export the folder structure, the user needs to select the folder where the zip-file is going to be created. Inside the zip-file there is:
a) The main circuit file in root, and
b) The loaded libraries required in a sub-folder called library
Hence a flat structure

[BFH-ktt1]

@MarcinOrlowski : Do you have time to review the PR's with milestone V3.7.0?

[MarcinOrlowski]

Yes, I will try.

[MarcinOrlowski]

I do not like the current behavior. No app acts that way so, as per my comment #1213 (comment) please consider using other name suffix. I proposed .lse but I wasn't thinking too long about that. Maybe we should not use .lse but something based on this i.e, .lsee (but two e looks bad) and keep lse for further, new save format instead? Or maybe this should become our defaut saving file format instead of .circ?

@maehne @davidhutchens @R3dst0ne 3rd opinion please?

[maehne]

Exporting a project should work like saving a regular ˋ.circˋ file, i.e., the file chooser should let the user enter the name for the project and ensure adding a distinctive file extension. We don’t need to limit ourselves to just three letters for the project file extension. The times of 8+3 letter file names to ensure MS-DOS compatibility are thankfully over since 25+ years. How about .lsprj or .lseprj? We should collect broader feedback to ensure choosing a good extension name with high acceptance and low conflict potential. The currently used .circ is very generic and may cause in some situations conflicts.

[davidhutchens]

I have a few comments to make considering where this seems to be going.

The idea of a le-package makes sense and would often make it easier to distribute things to students. We don't even use a 3-character extension in .circ, so using more is not an issue.

Having an Export command that produces an le-package suggests we ought to have an Import command that reads one. That was not really necessary when this was just producing a directory and opening the main .circ pulled in everything needed.

Making the current approach to packaging be the default may have some issues. It works great for a lot of use cases. But there are others that don't fit as well. Let me try to describe what I mean.

I have a large project that I used in teaching how a pipelined CPU works. It has a collection of parts in one .circ. I built a series of pipelines, adding features for each one. For example, one adds data forwarding, another adds pipeline stalls. Each of these uses the same parts .circ. Having different parts .circ files included with each save would have been difficult while building the series since modifications to the pipelines sometimes required modifications to parts. Keeping a single up-to-date parts .circ file meant I could easily back-test to keep all in sync.

When I built the .circ that provides caches, memory and I/O devices, I used the last in the series of pipelined CPUs in it. I also built a CPU that wasn't pipelined, but was actually microprogrammed (yes, that takes us back a few decades). But the pretty thing was that I could include both in the .circ file but only one of them was in use at a time. They were pin-compatible so it was easy to delete one and replace it with the other. This gave a lesson about maintaining an ISA over different implementations. Anyway, the save mechanism in Export throws away the "unused" .circ file rendering the quick replacement impossible.

I may not have seen the right way to use the packaged files, but this was all very easy when I controlled the directory structure and loaded .circ files. My point is we need to think broadly about use cases before moving to a packaged save as the standard one.

Thinking about this, I wonder if it would be appropriate to give a list of unused but loaded things that are about the be removed in the Export, and give the user the option of including them anyway. That doesn't fully address all the above, but might fix the "unused" implementation issue. The downside is it complicates the export process for the user.

[BFH-ktt1]

@MarcinOrlowski , @davidhutchens , @maehne :
I think that we should be clear what the function of this export is. When I implemented it I had following usage in mind:
a) The ability for me to provide students with a template for their practical works.
b) The ability for the students to provide a "clean" solution of their practical works to me.
c) The standard for logisim would still be .circ-files and I and the students had to manually unzip the files in their preferred location on their hard drive.
Hence the .zip extension made sense here as it is commonly used for zip-files, and also the "cleaning" makes sense as it only contains the parts used.

Looking the discussions I agree that point c) above could be replaced by a project-import action, and in this case also the .zip extension can/should be dropped. Furthermore, I agree with @maehne that the user should be able to name the exported project.

Than concerning to move completely to the zip-format as default, this I think is also doable (we already have an automatic "scratch-pad" on disc, namely the workspace of the fpga-commander), but in this case stripping should be disabled (see use case of @davidhutchens ).

I'll try to work a bit on it, but it will not be in V3.7.0.

I'll turn this PR into draft.

For the file extension I'm open

[MarcinOrlowski]

Having an Export command that produces an le-package suggests we ought to have an Import command that reads one.

Would that (partially) solve the confusion if the menu item read Save as project bundle... instead of Export...?
Also I used the name bundle here for a reason as this may be more than just a project as it is now. As for the file suffix, what about .lsebdl (I personally prefer shorter suffixes, but we shall stick to use .lse* in our files to directly indicate it's LSe file. Also it should not conflict with other apps.

[MarcinOrlowski]

The ability for me to provide students with a template for their practical works.

Just a kind reminder that if we aim for building best edu tool, we should be aware not only of teachers, but mainly the students. We should ensure that we offer not only the tools to complete the class but the can also save/export/submit they work with ease. That's why I really appreciate @BFH-ktt1 take on export feature to make sharing complete project easier.

BTW: I just realized that it might be helpful to have project's meta data (like, "owner", "title", "subject", "keywords", "description") to let people better personalize their files. It again, would be useful for students too.

[BFH-ktt1]

Would that (partially) solve the confusion if the menu item read Save as project bundle... instead of Export...?
Also I used the name bundle here for a reason as this may be more that it is now. As for suffix, what about .lsebdl (I personally prefer shorter suffixes, but we shall stick to use .lse* in our files to directly indicate it's LSe file. Also it should not conflict with other apps.

In this case we should also associate the .lse* files with Logisim evolution in the generated executable. I have no idea how to do that. For the moment I will take your proposed .lsebdl as extension (we can change that later if required).

[MarcinOrlowski]

files with Logisim evolution in the generated executable

This can be set up while creating installation packages. See support/*/file.jpackage and Flatpak XML config in https://github.com/logisim-evolution/logisim-evolution/tree/develop/support

[BFH-ktt1]

@MarcinOrlowski : I addressed the two points discussed. I know that the manifest reader is not the most beautiful thing that there is, and that also not all files are listed in the manifest. This I will "cleanup" in a new PR. For the moment I just want to get this in and push V3.7.1 out. Can you agree on the current state?

[MarcinOrlowski]

Thanks for the changes but the bundle loader still needs some work to make it user usable. Here are spotted issues:

1. temporary folder should be created w/o asking in system temporary storage
2. bundle file should be unpacked (if we cannot read directly from the zip w/o unpacking),
3. temporary folder should be removed.

For now you are unable to reuse user specified folder because there are leftovers from previous load.

Also:

4. There should be no "Open the imported project in Logisim-evolution" dialog. It's user intent to import thus open the file.
5. Imported project should always open new window without user being asked, even if currently opened one is empty, not in dirty state etc. That's how most apps work so user won't be surprised.

Could you at least address 1 (i.e. via Files.createTempDirectory("tmpDirPrefix").toFile().getAbsolutePath();) and then recursive delete for 3? The 4, 5 can be fixed later as this is just UX annoyance, while 1 and 3 blocks reuse of folder w/o manual cleanup (which requires using other tools or terminal)

[BFH-ktt1]

Sorry, I do not agree at all with your points 1-3, as:

a) Any changes to the project will not be stored in the project bundle (it is not the default file-format for logisim).
b) The project bundle is there to transfer the contents, hence it is intended to be extracted such that users can work on it (think as a template for exercises, intermediate transfer of parts of a project, etc.), deleting the temporary folder afterwards will erase all contents they added.
c) The project bundle is not a "read only" contents, hence if the user intends to import it, it is because they either want to work on it further or to test it out.

I can agree to 4) and 5) and directly open it.
Hence for me 1) .. 3) are points that are valid if you see the project container as a "read-only" contents which it is clearly not, and therefore I will not address 1)...3).

[MarcinOrlowski]

I am not against your intent, but I do not like the current implementation, which IMHO is against common practices and informal standards thus being potentially counter-intuitive for end user. Approving it as it works now, would (again, IMHO) significantly increase technical debt and I think we already have enough :)

If we assume that *.circ is LSe native file format then bundle export/import is just a helper functionality. This ain't unique among existing software. There other applications that offer export/import using non-native file formats. In case of importing/loading data from such non-native format the project is being imported in-memory only. Now if user would like to do any changes and save his works s/he can either choose to save in native format (with all the potential strings attached) or export in any of supported format. LSe should be no exception. If you import the project from the bundle, do some changes and then will attempt to save it, then choosing Save (CTRL+S) should not work, because project was not loaded (item should be disabled). So user would be forced to either Save As... or Export project bundle. It may be useful (maybe) to add Export bundle that will be only enable if project was imported from bundle, so that would work exactly as Save, overwriting source bundle file. But that's should it as this is what user experienced elsewhere thus s/he is expecting such behavior over anything "original". If *.lsebdl is a vessel, then once you left it, it should be "gone" unless you intentionally "in" again.

So again, I am OK with the PR once 1/3 in question are addressed for the reason given above. And to avoid protracted stand-off in this matter, I think we need additional opinions: @davidhutchens @maehne

[BFH-ktt1]

@MarcinOrlowski : the solution to the problem is very easy, I rename the function to Load and extract project bundle instead of Import project bundle, and than the user knows exactly what the action is which to expect. The Import project bundle functionality, as described by you also makes sense, however, has a big impact on many parts of the code (enabling/disabling menu entries, making logisim load from a zip-file instead of the file-system, have the possibility to make the project read-only on import, etc.) Many of these handles need to be implemented correctly to not have again a technical debt. Hence implementing your point 1..3 is more involved as just saving it to a temporary directory and afterwards deleting it. Furthermore, we than also have to decide if the import is read-only (users can play around with, but not add/modify) or modifiable with write-back to the bundle or an export to the file-system.
I would opt for an extra PR for the import.
What do you think?

[MarcinOrlowski]

Would be great for this to be a solution or workaround but it is not. Please consider the following scenario as the example:

1. user has two files 1.lsebdl and 2.lsebdl
2. he imports 1.lsebdl choosing /tmp (or Winddows/macOS equivalent) as his temporary folder (also because he do not understand the implications, as its counter-intuitive)
3. now s/he tries to import 2.lsebdl, choosing, again, /tmp as temp folder
4. this do not work.
5. User do not know why really. s/he just wanted to import from bundle file.
6. s/sh spreads the word LSe is "stupid and sucks" :)

Please note that *.lsebld is NOT equivalent of project folder because LSe do not use project folders. So either we introduce (for no real reason) that major change, or we will deal with bundles as user expects. And I'd bet all your money, that s/he expect it to be just a bundle. A file that holds complete project (whatever that means). And this is my point still and, unfortunately I still stand strong by my opinion that current implementation works counter-intuitive and against user expectations and should not be merged in current form.

[BFH-ktt1]

@MarcinOrlowski : Sorry to say, but your discussion is contradictory as
The function now is not import, but extract and open, hence 1.lsebdl will be extracted and opened, as well as 2.lsebdl, but they will not be imported in the current project.
I completely agree with you that import is another functionality, here there are no doubts, and hence needs to be implemented differently.
Also A file that holds complete project (whatever that means) is completely correct, and hence can be extracted and opened as the user it wants. This is neither counter-intuative, nor against user expectations, as it is the same functionality as taking winzip and extracting the contents of the zip-file to your file-system and than click on the main circ-file.

Hence I agree completely with you that import needs a complete other approach and is the completely wrong word for the current implemented functionality, but I do not agree with you that extract and open is counter-intuative, nor against user expectations.

I hope you understand my rambling

[MarcinOrlowski]

First, I do not consider this thread as rumbling. For me it a discussion heading for the best possible solution. And let me remind, that there's nothing that prevents you from saying "FY Marcin, I am merging", yet we still not there :) which let's me believe we share the same fundamental goals in this project. BTW: if you look back this massive thread you will notice that this discussion is what we should have before any implementation works even started. That approach would save lot of time wasted on implementation reworked later. I believe we have this discussed at some point that no PR should come without having ticket discussed first and this is a living example why we really shall enforce this policy.

Anyway, even you name menu item extract and open it would be still extremely odd looking. No other app does that. Please understand that the best U/X is the U/X that even dumbest user can approach. And don't get me wrong - it is not about stupidity of the users. It is about simplicity of the U/X. The simplest the tool the more user can focus on her/his task and not on how to achieve it. As a tool we should be as transparent as possible here. Imagine you are going to hammer the nail. Would you really like to first be forced to read the booklet to ensure you hold it right otherwise things could go wrong?

I do not want to go into excessive discussion here as it costed us both too much time already, so to sum it up: I do not consider current bundle support is "user ready". I expressed my view, concerns and expectations above. I got no clue how to solve the current stand-off right. I will try to think it over once more as, again, I like the feature. I just do not think it should be released in current state.

Let me again summon @davidhutchens @maehne for help.

[BFH-ktt1]

@MarcinOrlowski : Would you agree with this PR when I disable for the moment the Extract and Load menu item and make an issue?

[MarcinOrlowski]

I think I can accept merging even in current form under the condition import is not available. Not sure I'd push 3.7.1 with export feature enabled, but that another question, and I can also agree to it iff there's legitimate interest or needs for having it released for any reasons.

Please create a ticket for import feature with +1 priority.

[MarcinOrlowski]

Approved under discussed conditions.

[MarcinOrlowski]

@BFH-ktt1 what the faith of this one?

[BFH-ktt1]

@MarcinOrlowski : WIP, for the moment I have very limited time, will take it up later on