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

All text within square brackets is getting stripped from labels in the browser - Issue with BB code strip regex #223

Closed
arigit opened this issue Jan 31, 2017 · 7 comments
Labels
Milestone

Comments

@arigit
Copy link
Contributor

arigit commented Jan 31, 2017

Chorus 2 is able to navigate advanced playlists and the functionality works great, but there is one glitch.

I have the following playlist (amongst many others)

./kodi/userdata/playlist/music/HDAudio.xsp

<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<smartplaylist type="albums">
    <name>[HDAudio]</name>
    <match>all</match>
    <rule field="album" operator="contains">HDAudio</rule>
    <order direction="ascending">album</order>
</smartplaylist>

This playlist shows in the local machine GUI and also in my smartphone via Yatse, at the top of the playlist list, as "[HD-Audio]". When clicked, many albums in the collection match it.

In Chorus2 (2.4.1), when I list the playlists in Browser > Playlists > Music , I see an "empty playlist" at the top with a description: "no media in this folder", and this line can't be clicked.

It seems that Chorus2 has a problem with the playlist name containing non-letter characters. The reason for the non-letter first chars is that I want this to show at the top of the playlist list, as it does in the GUI and Yatse.
Hope this could be fixed in time for krypton's final release :)

@jez500
Copy link
Collaborator

jez500 commented Feb 1, 2017

There is a regex that removed BB code from labels which addons annoyingly use, it seems as though it thinks your title is a BB code tag (because it is in square brackets).

Short term fix... rename your playlist to use round brackets?

Long term fix... myself (or someone better at regex than me) see what is going wrong with this
https://github.com/xbmc/chorus2/blob/master/src/js/helpers/global.js.coffee#L175

@jez500 jez500 added the bug label Feb 1, 2017
@jez500 jez500 changed the title Advanced Music playlists fails with .xsp file All text within square brackets is getting stripped from labels in the browser - Issue with BB code strip regex Feb 1, 2017
@arigit
Copy link
Contributor Author

arigit commented Feb 1, 2017

I looked into this first from the POV of regular expressions (it's challenging to attempt to parse BBcodes with a regex as we know), but now I am wondering why there is a need to strip BBcode in a playlist name? I tried to find references on whether Kodi actually supports this at all but can't find any.

So bottom line, is there a need to strip bbcodes in this case to begin with? I can see other interfaces (coming back to Yatse) don't deal with bbcodes in playlist names

@jez500
Copy link
Collaborator

jez500 commented Feb 1, 2017

Yes there is a need. Google music addon for example has something like [i][color=orange]Next[color][i] for its next label and it doesn't look good. Yatse strips the BB code - but obviously has a more effective regex.
It is doable, just need to spend a bit of time on it

@arigit
Copy link
Contributor Author

arigit commented Feb 2, 2017

Understood

I think the key is that the regex needs to only match if the start of the string in square brackets is a valid BBcode expression, as opposed to other non BBcode content such as in my case. Hence this alternate approach seems to fit better:

http://stackoverflow.com/questions/2744423/javascript-bbcode-parser-recognizes-only-first-list-element

which is in fact multiple chained regexes like so,

  /\[b\](.*?)\[\/b\]/g,  
  /\[i\](.*?)\[\/i\]/g,
  /\[img\](.*?)\[\/img\]/g,
  /\[url\="?(.*?)"?\](.*?)\[\/url\]/g,
  /\[quote](.*?)\[\/quote\]/g,
  /\[list\=(.*?)\](.*?)\[\/list\]/gi,
  /\[list\]([\s\S]*?)\[\/list\]/gi,
  /\[\*\]\s?(.*?)\n/g);

This apporach should still address the google music case, and handle [HDAudio] like a champ.

@jez500
Copy link
Collaborator

jez500 commented Feb 2, 2017

This works /\[\/?(?:b|i|u|s|left|center|right|quote|code|list|img|spoil|color).*?\]/ig and it's one line ;)

https://regex101.com/r/Z89KU8/2

Will be updated in the next release

@jez500 jez500 added this to the 2.4.3 milestone Feb 2, 2017
@arigit
Copy link
Contributor Author

arigit commented Feb 3, 2017

That looks much better :)
Thank you for this

jez500 added a commit to jez500/chorus2 that referenced this issue Feb 21, 2017
@jez500
Copy link
Collaborator

jez500 commented Feb 21, 2017

Should be good in 2.4.3 👍

@jez500 jez500 closed this as completed Feb 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants