Skip to content

File System Watcher + Playlist Improvements#336

Merged
nlogozzo merged 16 commits intomainfrom
watcher
Sep 17, 2023
Merged

File System Watcher + Playlist Improvements#336
nlogozzo merged 16 commits intomainfrom
watcher

Conversation

@nlogozzo
Copy link
Copy Markdown
Member

@nlogozzo nlogozzo commented Sep 16, 2023

Closes #322
Closes #332

Here's the new file system watcher:
image

Here's the new create playlist dialog:
image

Also added a library mode indicator icon to the bottom bar:
image
image

@nlogozzo nlogozzo marked this pull request as ready for review September 16, 2023 02:40
@nlogozzo nlogozzo requested a review from fsobolev September 16, 2023 02:40
@nlogozzo
Copy link
Copy Markdown
Member Author

C#'s FileSystemWatcher on Linux is backed by libnotify...can confirm that no extra flatpak permissions are needed for this to function correctly, works with no problems.
NOTE: GNOME flatpak runtime contains libnotify so no worries there either.

@nlogozzo nlogozzo marked this pull request as draft September 16, 2023 03:01
@nlogozzo nlogozzo changed the title File System Watcher File System Watcher + Playlist Improvements Sep 16, 2023
- CreatePlaylistDialog asks for path to save through file picker
- Added support for creating and reading playlists with relative paths
@nlogozzo nlogozzo requested review from fsobolev and removed request for fsobolev September 16, 2023 03:37
@nlogozzo nlogozzo marked this pull request as ready for review September 16, 2023 03:37
@fsobolev
Copy link
Copy Markdown
Member

C#'s FileSystemWatcher on Linux is backed by libnotify

This doesn't make sense to me, libnotify is a library to send desktop notifications 😕

One more thing I want to change is filter in the file dialog can be used to select format instead of a separate comborow, I will work on that later today.

@nlogozzo
Copy link
Copy Markdown
Member Author

This doesn't make sense to me, libnotify is a library to send desktop notifications 😕

inotify sorry 😅

@nlogozzo
Copy link
Copy Markdown
Member Author

One more thing I want to change is filter in the file dialog can be used to select format instead of a separate comborow, I will work on that later today.

I don't know i feel like all the formats arent exposed to the user this way.

What we can do is if the user specifies the format in the file dialog (by providing the extension) then we can automatically select it in the format row.

@fsobolev
Copy link
Copy Markdown
Member

I don't know i feel like all the formats arent exposed to the user this way.

What we can do is if the user specifies the format in the file dialog (by providing the extension) then we can automatically select it in the format row.

I don't get how they are not exposed, a dropdown in the dialog doesn't differ much from a comborow, but this way users will set filename with extension and the format in one place, and this will prevent situation when file extension doesn't match the format.

@nlogozzo
Copy link
Copy Markdown
Member Author

One more thing I want to change is filter in the file dialog can be used to select format instead of a separate comborow, I will work on that later today.

Will the filter in the FileDialog only filters out files of that type. If the user isn't overwriting a file and just specifies the name without the extension, it doesn't get added on automatically...it's our job to do that...so we need to know the format the user wants still.

@fsobolev
Copy link
Copy Markdown
Member

If the user isn't overwriting a file and just specifies the name without the extension, it doesn't get added on automatically...

And then the validation should fail because the format is unknown. I think it's expected to always save playlist file with extension.

@nlogozzo
Copy link
Copy Markdown
Member Author

And then the validation should fail because the format is unknown. I think it's expected to always save playlist file with extension.

It's expected to save a playlist file with an extension yes, but we can't always expect the user to provide said extension as that's the nature of FileDialog...for example they may think that choosing the filter through the dialog is enough for choosing the format.

Similar how in Denaro a user can never sepcificy .nmoney at the end of an account file, and we add it on manually.

That's why I suggested that we keep the format row, but if the user specifies an extension in the file dialog we can change the format row to the extension they chose, otherwise they can still choose the format from the row and we will add the extension manually if needed.

@fsobolev
Copy link
Copy Markdown
Member

we can't always expect the user to provide said extension as that's the nature of FileDialog...for example they may think that choosing the filter through the dialog is enough for choosing the format.

Ah, I understand now, okay.

That's why I suggested that we keep the format row, but if the user specifies an extension in the file dialog we can change the format row to the extension they chose, otherwise they can still choose the format from the row and we will add the extension manually if needed.

Okay, will do this then.

@nlogozzo
Copy link
Copy Markdown
Member Author

Okay, will do this then.

I could take care of this should just be a couple of lines and I'll time during the atternoon I believe to do it :)

@fsobolev
Copy link
Copy Markdown
Member

fsobolev commented Sep 16, 2023

we can't always expect the user to provide said extension as that's the nature of FileDialog...for example they may think that choosing the filter through the dialog is enough for choosing the format.

Huh, it seems like it depends on file chooser. Here on KDE it always adds .m3u, probably because it's the first format in the filter. Even if I write another supported extension, .m3u will be added anyway 🤔 So... Let's talk about the idea of setting the format in file dialog? 😄

Also, FPL should be removed from the list.

FPL writing not implemented
   at ATL.Playlist.IO.FPLIO.setTracks(FileStream fs, IList`1 result)
   at ATL.Playlist.PlaylistIO.setTracks(IList`1 trackList)

@nlogozzo
Copy link
Copy Markdown
Member Author

Huh, it seems like it depends on file chooser. Here on KDE it always adds .m3u, probably because it's the first format in the filter. Even if I write another supported extension, .m3u will be added anyway 🤔

If you change the filter in the KDE dialog does it add the right extension?

So... Let's talk about the idea of setting the format in file dialog? 😄

Well what I have in mind for fixing this will handle both these cases on Gnome and KDE

Also, FPL should be removed from the list.

👍, however, i'll open an issue on ATL because it's listed there as supported.

@nlogozzo
Copy link
Copy Markdown
Member Author

however, i'll open an issue on ATL because it's listed there as supported.

Zeugma440/atldotnet#230

@fsobolev
Copy link
Copy Markdown
Member

If you change the filter in the KDE dialog does it add the right extension?

I will check, it should, currently we have only "All formats" so it was using the first one inside it

@nlogozzo
Copy link
Copy Markdown
Member Author

If you change the filter in the KDE dialog does it add the right extension?

I will check, it should, currently we have only "All formats" so it was using the first one inside it

Ah I thought I added all individual ones as well. Will do that and the fixes I had planned and we can check after if that solves everything. Just be a little bit till I get home from work...busy day today 😅

@fsobolev
Copy link
Copy Markdown
Member

If you change the filter in the KDE dialog does it add the right extension?

If I choose individual filter then an extension is added correctly.

@nlogozzo nlogozzo requested review from fsobolev and removed request for fsobolev September 17, 2023 03:08
@nlogozzo
Copy link
Copy Markdown
Member Author

@fsobolev Okay try latest commit, should all be good!

@nlogozzo nlogozzo merged commit 25a9174 into main Sep 17, 2023
@nlogozzo nlogozzo deleted the watcher branch September 17, 2023 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Playlist Improvements File System Watcher

2 participants