Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
51 stars 10 forks source link

removing artist name from the song title when playing from youtube #52

Closed alexou2 closed 1 year ago

alexou2 commented 1 year ago

gh told me to create an issue in order to fork the project. How do I test if the feature I want to add works or not?

Batwam commented 1 year ago

With the current stable version, as a lot of youtube videos include the artist name and the browser will feed the video title to title, you might be getting the artist twice. Is this what you mean? if you change the fields from Artist/none/title: image to title/none/none, it will remove the duplicated artist at least: image

We also have an open pull request for advanced field filtering using regex here: https://github.com/Moon-0xff/gnome-mpris-label/pull/31 this would probably achieve what you are looking for if you want to try this, however, the filtering isn't source specific: https://github.com/Batwam/gnome-mpris-label-batwam/tree/filtering_new image

alexou2 commented 1 year ago

Ohh... Is there something else that i can do to contribute to the project?

Batwam commented 1 year ago

You can certainly test that branch and see if it does what you need. It was initially intended the extend the ability to filter some types of text like "featuring" "live at ..." which make the titles too long in a way people can customise. I am not 100% of if it does what you are looking for?

We'd be interested to hear about users about whether it works or not, what part they find useful/confusing, etc...

alexou2 commented 1 year ago

I was just looking for a way to help in the project in some way, since i like how it works

alexou2 commented 1 year ago

how do I get the menu where I can filter the name? I tries cloning the fork you mentioned, but I got the same interface as before

Batwam commented 1 year ago

Did you log out and log back in to reload the extension? In X11, you can also type alt+F2 and r to restart gnome-shell.

alexou2 commented 1 year ago

I logged out and back in and it didn't work. I cloned the fork and then ran the install script. gnome extensions also gave me a notification saaying that there was an update

Batwam commented 1 year ago

The extension manager will overwrite the files in the background if it finds that they have a lower version number than what is available on extensions.gnome.org. It shouldn't happen though as they should both be version 14.

Anyway, I updated metadata.json to version 15 in the fork. Could you try again?

alexou2 commented 1 year ago

I cloned the fork but the version didn't update and I still have the issue git clone https://github.com/Batwam/gnome-mpris-label-batwam.git && cd gnome-mpris-label && ./install.sh

Batwam commented 1 year ago

That's because you are in the main branch by default which is a duplicate of this repo's main. Do a git checkout filtering_new to go to the branch from the pull request.

alexou2 commented 1 year ago

thx now it works.

Batwam commented 1 year ago

Does this do what you want? Please report any feedback in #31

alexou2 commented 1 year ago

actually no. what I wanted to do was to have a regex that would do something like that title.replaceAll(artistName, "");. is there documentation on how the code works?

Moon-0xff commented 1 year ago

what I wanted to do was to have a regex that would do something like that title.replaceAll(artistName, "");

From the regex field of that branch that's not possible, all it does is apply the provided regex to the title/artist/album string.

What you want to do could be achieved by editing the label.js file.
label.js was designed with hacking in mind, it's a short file, well commented (i hope), and the extensions passes the entire players interface to it (defined in players.js).

Is there documentation on how the code works?

No, but the harder to read parts are commented, variable names are descriptive, and as we developed most of the features as a duo the PRs are full of information about the inner workings of the extension.

I hope the prerequisites for editing label.js are just programming knowledge, and reading the players.js file, but admittedly that file is not very clear, and it's short on comments.

There's also the issue that web browser don't pass information to the extension that could help distinguish between a youtube source and a non-youtube source.

Moon-0xff commented 1 year ago

My initial idea would be to add a test case like:
if players.selected.address matches chrome's: add "Remaster" to the end of the LAST_FIELD string to the buildLabel function.

Assuming LAST_FIELD is title, and the remove-remaster-text option is enabled.

Batwam commented 1 year ago

Assuming the below format: image if playing music from youtube exclusively, it would be possible to exclude anything before - using the regex fork and regex syntax. That would the most basic solution but could be fit for purpose and not require touching any js code.

Based on the screenshot above, chrome/youtube appear to display the artist name image a neater solution could be to filter out artist from title by editing label.js and do some regex based metadata cleanup by subtituting artist within title with blank.

I think that this is too niche to implement in the main code but one thing worth considering would be to provide the ability to specify which sources the filters should apply to, similarly to the Ignore/Allowed lists?

Moon-0xff commented 1 year ago

I think that this is too niche to implement in the main code but one thing worth considering would be to provide the ability to specify which sources the filters should apply to, similarly to the Ignore/Allist lists?

That referring to #31, which I'm thinking of discarding altogether.

It's not that #31 isn't useful or a good addition to the extension, but it requires extra effort in our side, and regex knowledge the user's side.
And if the user knows how to write regex, isn't a stretch that could also edit the source code.

Batwam commented 1 year ago

@Moon-0xff let's discuss in #31

Batwam commented 1 year ago

Seems like this feature isn't going to be implemented as a little bit too niche but can be achieved using the regex filtering fork provided and/or editing the code. So, should we close this as won't fix?

Moon-0xff commented 1 year ago

Not at all! The issue is not about adding this change, but how to do it.
And we do indeed lack some sort of internal documentation and the code can be clearer and easier to edit in some places.

As a response I've added HACKING.md 2 days ago and I'm partially rewriting label.js so it's easier to edit.

(Do note that HACKING.md surely needs more work)

Moon-0xff commented 1 year ago

As mentioned, I added a guide to modify the extension (although still needs work) and rewritten the label.js to make changes like this easier to add.

I was thinking of leaving the PR open for a while as I was unsure if the code layout of the file was up to the task.

But I've added my idea in a few seconds so I think it's good enough.

Here's a diff for the changes, they are untested though

diff --git a/label.js b/label.js
index 6798f7b..4a5246b 100644
--- a/label.js
+++ b/label.js
@@ -46,6 +46,10 @@ var buildLabel = function buildLabel(players){
    let labelstring = "";
    fields.forEach(field => {
        let fieldString = stringFromMetadata(field,metadata); //"extract" the string from metadata
+
+       if(players.selected.identity == "Chromium" && field == "xesam:title" && fieldString.includes("-"))
+           fieldString += "Remastered";
+
        fieldString = parseMetadataField(fieldString); //check, filter, customize and add divider to the extracted string
        labelstring += fieldString; //add it to the string to be displayed
    });

$ patch < [file with the diff] while on the working directory to apply it.
Requires the latest main so I suppose you will want to do a $ git pull first

Moon-0xff commented 1 year ago

Closing this. The changes should aid to any newcomer to edit the extension (I hope). Some work is still needed to make HACKING.md a bit more useful, but overall it should be enough at least as a starting point.

Last comment might just answer the starting question (haven't tested it) but in any case I'm marking this as completed 😉