migrating Preferences and Messages to stand-alone version #1130
Conversation
|
so for the language display issue, i suspect it happened when i renamed it's fixed now |
|
Hi @AhmedMagedC Thank you for the PR! Sorry I got a little busy, hope to get to this soon! |
There was a problem hiding this comment.
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
| @@ -472,12 +471,12 @@ static public String getVersionName() { | |||
|
|
|||
|
|
|||
| public static void setCommandLine() { | |||
| commandLine = true; | |||
| System.setProperty("processing.cli", "true"); | |||
There was a problem hiding this comment.
Could you please explain why this needed to change?
There was a problem hiding this comment.
The main motivation behind this change was to be able to know if Processing is launched via CLI or not without the need to depend on app module via using Base.isCommandLine()
as for example im using it in the utils.Messages.kt
| */ | ||
| @JvmStatic | ||
| fun showMessage(title: String = "Message", message: String) { | ||
| if (Base.isCommandLine()) { |
There was a problem hiding this comment.
I would say that the JOptionPane parts should stay and the println can be in the universal class
There was a problem hiding this comment.
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
| load(new FileInputStream(preferencesFile)); | ||
|
|
||
| } catch (Exception ex) { | ||
| Messages.showError("Error reading preferences", |
There was a problem hiding this comment.
I think these could be transformed to normal errors and then catch and log them in the app classes
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Please add the build folder to the .gitignore
Resolves #1104
Changes
Preferencesto:app:utilsmodule making it independent ofappmodule by also moving the functionalities that depends onapptoAppPreferencesclassMessagesclass to be able to use it (for example in debugging) in different areas of ProcessingUtilsinto:app:utils, because why keeping it inappit doesn't need any functionalities from there, and also can be used in:app:utilswithout the need to depend onappgetSettingsFolder()method for each platform from:appto:app:utils, without duplication the code i madeapp.Platformclass to depend onSettingsResolverin:app:utilsto get the correct location for each platformdefaults.txtnow by using JAR resource system instead ofPlatform.getContentFile()(which is also marked as deprecated in the codebase encouraging to migrate to JAR RS)Todo
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