migrating Preferences and Messages to stand-alone version

[AhmedMagedC]

Resolves #1104

Changes

• migrating Preferences to :app:utils module making it independent of app module by also moving the functionalities that depends on app to AppPreferences class
• doing the same for the Messages class to be able to use it (for example in debugging) in different areas of Processing
• moving Utils into :app:utils, because why keeping it in app it doesn't need any functionalities from there, and also can be used in :app:utils without the need to depend on app
• now for the platform-dependent settings location, the previously mention approach in the issue made cyclic dependency problem, so i moved the getSettingsFolder() method for each platform from :app to :app:utils, without duplication the code i made app.Platform class to depend on SettingsResolver in :app:utils to get the correct location for each platform
• loading the defaults.txt now by using JAR resource system instead of Platform.getContentFile() (which is also marked as deprecated in the codebase encouraging to migrate to JAR RS)

Todo

• Adding callback events/listeners when a preference changes
• Tests that will declare and test the functionality of Preferences
• A Github Action that will run the Tests on all supported platforms

Tests

I tried to compile and run, as for now preferences are loaded and saved correctly

but for some reason some languages aren't displayed properly like: arabic , chinese, but languages like: english, japanese properly displayed.
I will look into it, but any help or suggestion of why that happens is very much appreciated

[AhmedMagedC]

so for the language display issue, i suspect it happened when i renamed Preferences class to AppPreferences
the comment: # Preferences (Frame) in all PDE.properties automatically changed to # AppPreferences (Frame) leading to corruption of some files (idk why tho)

it's fixed now

[Stefterv]

Hi @AhmedMagedC Thank you for the PR! Sorry I got a little busy, hope to get to this soon!

[Stefterv]

Hi @AhmedMagedC Thank you again for the work! It is heading very much in the right direction. I have added some comments as suggestions to help further improve

[Stefterv]

I would say that the JOptionPane parts should stay and the println can be in the universal class

[AhmedMagedC]

If i understand you correctly, you wanna seperate the appearance of graphical messages via JOptionPane to be only used by app.Messages and a simple println only for the utils.Messages

but wouldn't that make an issue in utils module? as for example in processing.utils.settingslocation and processing.utils.Base
error messages via Messages.showError() wouldn't display a graphical message only just a console print. So is it accepctable? also bear in mind that i cannot use app.Messages for this purpose because i want the utils module along with the utils.preference class to be fully independent of app module

[Stefterv]

I think these could be transformed to normal errors and then catch and log them in the app classes

[AhmedMagedC]

I can do that. but that would depend on how we choose to implement utils.Messages

if we decided to keep the use of JOptionPane in utils.Messages then this delegation of catching and logging the errors in app rather than in utils wouldn't make much sense because we will use the same method but in different location

if we decided to remove JOptionPane and keep only the println then i can do that delegation

[Stefterv]

Please add the build folder to the .gitignore

[AhmedMagedC]

already did in this commit

[Stefterv]

Yes but the build files themselves are still in the PR (see here for examples on how to clean this)

[AhmedMagedC]

Okay done!
Thanks for pointing this out, i though git would remove it automaticlly after adding it to .gitignore
what about the other points above?

[Stefterv]

Hi @AhmedMagedC Sorry it took me so long to take a look at your PR again! and again thank you for your efforts, it's helping me a lot trying figure out how to refactor some of these intertwined elements.

Looking at your PR and #1164 I think it is important moving forward that we only migrate one class at a time. I would suggest that any functionality in the Preferences class that is currently causing you to have to migrate the Messages and Base classes for example, should be taken out of the Preferences class. That will make the code a little bit more flexible in the future.

[AhmedMagedC]

Hi @AhmedMagedC Sorry it took me so long to take a look at your PR again! and again thank you for your efforts, it's helping me a lot trying figure out how to refactor some of these intertwined elements.

Looking at your PR and #1164 I think it is important moving forward that we only migrate one class at a time. I would suggest that any functionality in the Preferences class that is currently causing you to have to migrate the Messages and Base classes for example, should be taken out of the Preferences class. That will make the code a little bit more flexible in the future.

Hi @Stefterv , It's okay it's also the first time for me trying to refactor a codebase by decoupling tightly-coupled classes

im not sure i follow you,
if i were to move Messages back to app module, how might we decouple Preferences from using it?

also for the change of getting the setting folder for each OS is now independent of Base class, i dont think i should undo this change it will make Preferences impossible to be a standalone class

[Stefterv]

Yes exactly, instead of migrating Messages along with Preferences, app.utils.Preferences should throw an error that is caught within the encapsulating app.Preferences, same with the paths app.utils.Preferences could take a path as an argument on creation and that argument is set in app.Preferences

[AhmedMagedC]

Hi @Stefterv , Sorry for the late reply i was busy for the last couple of weeks

i see now that we should currently focus on migrating Preference class
i migrated Messages with it in the first place to be able to print the error messages
but as you suggested i can simply throw them then caught and handled in app.preferece class

for the paths, i dont undetstand why i should undo it
now i fully docoupled getting the preference file location from the app module
you can simply call SettingsResolver.getSettingsFolder(); which will get you the location without the need to depend on app module

[Stefterv]

Hi @AhmedMagedC! Again your work on this has been invaluable in figuring out a strategy for starting to untangle some of these parts.

I’m recommending we tackle one class at a time to avoid regressions and the risk of having to undo a PR because of an unforeseen side effect in the existing code.

For me, an important part of these decoupling steps is making the coupling between classes more visible. That way, we can gradually refine each class to have clearer inputs and outputs.