search

elementary / music

Music player and library designed for elementary OS
https://elementary.io
GNU General Public License v3.0
145 stars 49 forks source link

Allow to open directories directly #747

Closed ghost closed 1 year ago

ghost commented 1 year ago

These will be worked on a separate PR:

Fixes #709

ghost commented 1 year ago

Got it, will check it out again ASAP. Thanks for taking the time to review my code!

ghost commented 1 year ago

Ok, resolved. How do I lint the code? Ran what's on the CI, but it didn't work (vala-lint is not installed in my machine).

lenemter commented 1 year ago

Ok, resolved. How do I lint the code? Ran what's on the CI, but it didn't work (vala-lint is not installed in my machine).

vala-lint unfortunately doesn't automatically lint the code, it only shows lint errors. If you want you can install it from https://github.com/vala-lang/vala-lint

BAProductions commented 1 year ago

Ok, resolved. How do I lint the code? Ran what's on the CI, but it didn't work (vala-lint is not installed in my machine).

vala-lint unfortunately doesn't automatically lint the code, it only shows lint errors. If you want you can install it from https://github.com/vala-lang/vala-lint

Actual it does an I've done it

lenemter commented 1 year ago

Oh I didn't know there is a --fix flag. Nice discovery.

ghost commented 1 year ago
ghost commented 1 year ago

Fixed the style of code. For some reason, it works from the terminal but not from the file explorer: https://youtu.be/pNTw4lanGbo

lenemter commented 1 year ago

Fixed the style of code. For some reason, it works from the terminal but not from the file explorer: https://youtu.be/pNTw4lanGbo

That's happening because in this case it launches flatpak version of Music. You need to delete it through AppCenter or through Terminal by executing flatpak remove io.elementary.Music and then it should work.

ghost commented 1 year ago

Ok, need to make the file methods private and make one function inline and then it should be good to go.

ghost commented 1 year ago

The only thing that doesn't work for me is that by default, Music doesn't open folders, even though I set inode/directory. If set manually it works fine, though

lenemter commented 1 year ago

Well, it doesn't work :( @danirabbit You know more about flatpak, so maybe you have an idea why?

lenemter commented 1 year ago

Well, it doesn't work :( @danirabbit You know more about flatpak, so maybe you have an idea why?

Allowing access to Home folder works. But is there a better solution?

ghost commented 1 year ago

Isn't it expected that the app needs access to the filesystem? It should be on by default though.

jeremypw commented 1 year ago

This works when run natively but not when in a Flatpak (but master doesn't anyway). I find Files does give Music as one of the options in the "Open With" submenu. I am working on the Flatpak version atm. You can debug the program while running in a Flatpak by launching with flatpak run --command=sh io.elementary.music. This gives you a shell prompt inside the sandbox from where you can launch the program as usual but can see all the debugging and error messages as well as being able to run gdb if you need to.

jeremypw commented 1 year ago

@aitor-gomila I've pushed a PR to your fork with a small fix for Flatpak compatibility. With this I can open files and folders from Files with Music both by using the context menu and by drag-drop. Launching from terminal with flatpak run io.elementary.music <path to file> also works. All providing that the folders/files are within ~/Music of course. (or whatever XDG_MUSIC is pointing to). I would be grateful if you could merge it as I do not have permission to push to your PR directly.

ghost commented 1 year ago

Thanks. How did Music work without the changes on your PR?

jeremypw commented 1 year ago

Note that giving access to the home directory with filesystem=home does not give access to all its subfolders for some reason. But good practice is to allow each app access only to one folder (in this case Music) in addition to its own Flatpak folder (which the average user would not be able to find!)

jeremypw commented 1 year ago

Thanks. How did Music work without the changes on your PR?

It only worked if running natively - not if it was running as a Flatpak. For Files context menu to work without my fix you would have to install it natively as well.

ghost commented 1 year ago

Done. Thanks for taking the time to review my PR

jeremypw commented 1 year ago

One extra thing - the appdata for the next release should be updated with the new functionality.

ghost commented 1 year ago

Appdata?

ghost commented 1 year ago

misclick, sorry

jeremypw commented 1 year ago

Sorry - its called metainfo in this repository I see. Its the file /data/music.metainfo.xml.in. You will see other sections in there the last release was 7.0.1 in December so the new info needs to be added to the pending 7.1. Looks like drag and drop is already entered so the new functionality is to open folders in Music from Files. Don't worry - its not too important - it will be checked and updated as needed before the release is made anyway.

ghost commented 1 year ago

@lenemter Can you check if sudo make install installs system-wide or just for the current user? I created a separate account and I'm able to open folders.