Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate com.jidesoft:jide-components dependency #51

Merged
merged 7 commits into from
Feb 5, 2023

Conversation

Borewit
Copy link
Owner

@Borewit Borewit commented Feb 3, 2023

Eliminate com.jidesoft:jide-components dependency, aiming to be 100% open source.

Resolves #50.

ToDo:

  • Make tabs close-able

Test notes: The only thing which are relevant in this PR is the GUI.

@Borewit Borewit added debt Technical debt dependencies Pull requests which update a dependency file labels Feb 3, 2023
@Borewit Borewit self-assigned this Feb 3, 2023
@Borewit
Copy link
Owner Author

Borewit commented Feb 3, 2023

Build: listFix_2.5.1-[PR51]-3.exe.

Known issue: tabs are not close-able, workaround: close via JMenu.

@Borewit Borewit force-pushed the eliminate-jidesoft-dependencies branch from 4498734 to 1153736 Compare February 3, 2023 21:54
@Borewit
Copy link
Owner Author

Borewit commented Feb 3, 2023

Made the tabs close-able. Build: listFix_2.5.1-[PR51]-4.exe.

@touwys
Copy link

touwys commented Feb 4, 2023

@Borewit

Do you want listFix_2.5.1-[PR51]-4.exe tested, or listFix_2.5.1-16 ?

@Borewit
Copy link
Owner Author

Borewit commented Feb 4, 2023

If possible both. They both contain different changes. This is a different branch. Currently The trunk (default branch) of the tree is version 2.5.1. This PR describes the changes of this branch (relative to the trunk) which is about getting rid of commercial Jidesoft dependencies.

When you see no issues compared to 2.5.1, this one is going to be merged to the trunk (default branch).

If you do, I keep adding commits (changes) to this branch to fix.

So this is the pattern we follow, it starts with an issue describing a specific change (bug/enhancement/debt/dependency-update). Then we create a branch to resolve that issue. When te branch is completed it will be pushed to GitHub and can then be turned into PR(pull request). Also called a change request. So this essentially a request (proposal) to change the trunk. Typically the PR describes the change, and also refers to the issue it claims to resolve.

@Borewit
Copy link
Owner Author

Borewit commented Feb 4, 2023

Build with improved playlist closure handling: listFix_2.5.1-[PR51]-5.exe

@Borewit Borewit force-pushed the eliminate-jidesoft-dependencies branch from 32cb1df to b15af0b Compare February 4, 2023 10:59
@Borewit Borewit force-pushed the eliminate-jidesoft-dependencies branch from b15af0b to 74c6a8a Compare February 4, 2023 11:00
@touwys
Copy link

touwys commented Feb 4, 2023

listFix_2.5.1-[PR51]-4.exe

I tested one playlist only. Same result as with listFix_2.5.1-16::

Of the five closest matches found, three would play, and two would not [edit] react to input to play, at all.


rollingfile.log

Also, this build appears to have introduced a new issue, corrupted tab title-text:


Corrupted Tab Title Text

@Borewit
Copy link
Owner Author

Borewit commented Feb 4, 2023

I tested one playlist only. Same result as with listFix_2.5.1-16: Of the five closest matches found, three would play, and two would not [edit] react to input to play, at all.

So that must be an issue already present in version 2.5.1 and should therefor best to be reported as a new issue/bug.

@Borewit
Copy link
Owner Author

Borewit commented Feb 4, 2023

Also, this build appears to have introduced a new issue, corrupted tab title-text

I am so sorry, I cannot reproduce that issue, but I did find an issue closing tabs. I also tried different look & feels, but I had no issues in visualizing the tabs.

Latest build for this PR: listFix_2.5.1-[PR51]-7.exe

@touwys
Copy link

touwys commented Feb 4, 2023

To be on the safe side, I'm going for a clean installation of listFix_2.5.1-[PR51]-7

@touwys
Copy link

touwys commented Feb 4, 2023

I am so sorry, I cannot reproduce that issue, but I did find an issue closing tabs. I also tried different look & feels, but I had no issues in visualizing the tabs.

I have just now installed listFix_2.5.1-[PR51]-7 — a clean installation:

  1. Added Media Directory. It loaded much, much quicker than before.

  2. Opened Options and Set/Add Playlist Directory.

  3. Opened the playlist to check Preview>Play. Quote:

Of the five closest matches found, three would play, and two would not [edit] react to input to play, at all.

  1. All five tracks now performed equally well.

  2. The tab header-text is still corrupted, over here — even after the clean app installation, which reverted to the default font for the app. [Edit: Also, I opened ca. 12 tabs in the tab bar, some of which were possible to close, and some did not].

Note 1 on 5: I reasoned that this text corruption issue might, perhaps, be related to the font style and size, so I wanted to test the theory by changing the font style and size in the app Options. To my surprise, the app hung after choosing the new font style, and had to be forcefully closed, before restarting.

Note 2 on 5: The corrupted text, with the two exclamation marks, actually looks like two tabs superimposed one upon the other?

Note 3 on 5: Have you perhaps tried different font style? My usual default is Candara_14


rollingfile.log

@Borewit
Copy link
Owner Author

Borewit commented Feb 4, 2023

I opened ca. 12 tabs in the tab bar, some of which were possible to close, and some did not].

A yes, that issue I have as well, only the last active tab you are allowed to close 🤣

@Borewit
Copy link
Owner Author

Borewit commented Feb 4, 2023

Candara 14, changed that on the fly:
image

image

But I override the default tab visualization component, so I think for some reason on your end, the old one is also there. Let me try something...

@Borewit Borewit force-pushed the eliminate-jidesoft-dependencies branch from 6012b7d to 27d8e19 Compare February 4, 2023 23:54
@Borewit
Copy link
Owner Author

Borewit commented Feb 5, 2023

listFix_2.5.1-[PR51]-9.exe fixing closing tabs, and maybe the cluttered tab text?

@Borewit Borewit force-pushed the eliminate-jidesoft-dependencies branch from 27d8e19 to 5f59752 Compare February 5, 2023 00:34
@touwys
Copy link

touwys commented Feb 5, 2023

listFix_2.5.1-[PR51]-9

Sorry for the delay. I was determined to find something sensible to report on. Hopefully, you will gain something worthwhile from the results below. The following is a reasonable, if not perfectly accurate, reflection of the steps that were followed in testing this build:

  1. Tried installing and reinstalling listFix_2.5.1-[PR51]-9 several times, overwriting the previous installations.

——1.1 The issues, previously reported, persisted.

  1. Prepared for a clean installation of listFix_2.5.1-[PR51]-9 by first removing completely all the files associated with the listFix installation. ("Revo Uninstaller Pro" was used for the job.)

——2.1 The issues, previously reported, persisted.

  1. Searched for possible leftovers, such as an app configuration file, that could possibly be susceptible to content corruption.

——3.1 Discovered "C/Users/User/.listFix()position.ini". The uninstallers seems incapable of locating this file for removal.

  1. Deleted ".listFix()position.ini", and installed listFix_2.5.1-[PR51]-9 anew.

  2. Restarted the app, and set the Playlist and Media Directories.

——5.1 This operation completed successfully.

  1. Selected 12 x playlists from the Playlist Directory, and opened them all in tabs in the playlist edit pane.

——6.1 BINGO!

——6.2 Repair operations — "Find Exact Matches" & "Fix Everything" (i.e. "Find Closest Matches") — were initiated for two of the playlists. Both operations completed successfully. The Preview>Play operations also went well, and smoothly so.

  1. Opened "Options", and changed the font type, and size, twice before reverting to the default.

——7.1 No issue found.

  1. Closed several open playlist tabs by clicking on the blank space where the "x" used to be. (See 6.1)

——8.1 I cannot rightly, or precisely, put my finger on it, but this process delivers "awkward" results, which are difficult to explain without further investigation.

  1. Opened the app "File" menu, and selected "Close All" (Ctrl+Shift+W) with which to close all the open tabs at once.

——9.1 All the tabs closed expediently and properly.

  1. Selected again the same 12 playlists as before, to open for repairs. (See 6.0)

——10.1 Issue reappeared: all the tab titles were now again corrupted.

  1. Closed listFix.

  2. Located and deleted the "C/Users/User/.listFix()position.ini" file.

  3. Restarted listFix().

——13.1 The closing configuration remained intact.
——13.2 Where are the app config settings stored?


PS. If already discarded, why should the "logfile.log" file still be present in the app logs? Also, in the "rollingfile.log" below, does not seem to contain any valuable information?

rollingfile.log

@Borewit
Copy link
Owner Author

Borewit commented Feb 5, 2023

Thanks for you thorough review @touwys

I don't believe much of the requirement to do deep clean. Uninstalling old Java versions, updating video drivers, although I don't think it will help, but have a higher probability to make a difference.

May I summarize your observations like this:

  1. Opening 12 or more tabs may cause the close tab butting to vanish.
  2. Awkward behavior closing tabs
  3. Opening and closing tabs after a while causes tab-component to render incorrectly, which may be relater to 3.

.listFix()/position.ini

With the migration to from .ini files to .json I forgot the position.ini. I seems to used to open the playList() Window on the last position with the same size. Created issue #54 to migrate that one as well. In principal you should be able to delete those file and they will be re-generated.

• PS. If already discarded, why should the "logfile.log" file still be present in the app logs? Also, in the "rollingfile.log" below, does not seem to contain any valuable information? • [rollingfile.log]

I agree, not relevant at this time. Only when you experience crashes or the application hangs.

13.2 Where are the app config settings stored?

All application data is stored in C:\Users\user\.listFix()\*. You can open all files with a text editor.

Even if there some side-effects, I am in favor of merging this PR as having commercial software in here just no good. Even more I know now the added value was very limited. (I the process of removing, surprisingly I ended up with more then 200 lines of code less) . If required, I can remove the override of the tab visualization, and find an alternative way to close the tab.

@Borewit
Copy link
Owner Author

Borewit commented Feb 5, 2023

I cannot reproduce the issue, opened a large number of playlists, and closed all of those, and then opened then again, repeated that a few times, no side-effects:

image

Change rollover color cross to blue.
@Borewit Borewit force-pushed the eliminate-jidesoft-dependencies branch from 4fec32d to 71b7237 Compare February 5, 2023 12:13
@Borewit Borewit merged commit 1295dc5 into main Feb 5, 2023
@Borewit Borewit removed the debt Technical debt label Feb 5, 2023
@touwys
Copy link

touwys commented Feb 5, 2023

I cannot reproduce the issue, opened a large number of playlists, and closed all of those, and then opened then again, repeated that a few times, no side-effects:

We're missing something here. It seems like this issue is due to one or other file corruption. It does work properly with a clean installation, and that *.ini file removed. What causes this issue is yet to be established. It seems to be something that was introduced in later builds.

Just now, I deleted all the json settings files, restarted the app and set it up all over again. Then reopened the 12 playlists for repair, and yet their titles were still contaminated. It looks like two superimposed titles, as I explained before.

What happened next, is surprising, I opened the Options dialogue, and tried to change the number of closest matches to find. The app immediately started to hang... I moved the indicator around... video:


Tab Text Contaminated & App Hangs @ Options

@touwys
Copy link

touwys commented Feb 5, 2023

Even if there some side-effects, I am in favor of merging this PR as having commercial software in here just no good. Even more I know now the added value was very limited. (I the process of removing, surprisingly I ended up with more then 200 lines of code less) . If required, I can remove the override of the tab visualization, and find an alternative way to close the tab.

You have done very, very well so far, thank you. Follow your head, but just please don't give up now. I think if you can help sort out the contaminated tab-titles issue (your ref: "tab visualization"?) over here, we've got a nice workable app. Looking ahead, if you still feel up to it, there are still a handful of enhancement proposals waiting for you. These are not aimed at improving the current operations — because those really are fine — but more like to improve the work flow.

@Borewit
Copy link
Owner Author

Borewit commented Feb 5, 2023

I am not giving up.

Please note I created a new issue, with specific focus that problem @touwys, see #55.

This PR is now merged.

@touwys
Copy link

touwys commented Feb 5, 2023

THANK YOU

If you want to see one of my proposals in the meantime, where do you want me to put it for you?

@Borewit
Copy link
Owner Author

Borewit commented Feb 5, 2023

If you want to see one of my proposals in the meantime, where do you want me to put it for you?

Preferably in dedicated issues. That way I can follow up with focused PR's, and you focus on just testing on specific changes.

PR #47 has now a lot of outstanding (refactoring) work, so if that PR does not introduce new issues, I like that very much to be merged.

@Borewit Borewit deleted the eliminate-jidesoft-dependencies branch February 11, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests which update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eliminate JIDE Software dependencies
2 participants