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

Template doesn't format at all when contains a comma #2166

Closed
miblodelcarpio opened this issue Aug 10, 2016 · 7 comments
Closed

Template doesn't format at all when contains a comma #2166

miblodelcarpio opened this issue Aug 10, 2016 · 7 comments
Assignees
Labels
bug bugs that are confirmed and actionable

Comments

@miblodelcarpio
Copy link

Problem

Including a comma in a path yields incorrect path.

Running this command in verbose (-vv) mode:

$ beet -vv ls -f 'Music/Pajo, David/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title' Pajo

Led to this problem:

Music/Pajo, David/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title

Simply removing the comma in Pajo, David yields the expected result.

Setup

  • OS: Arch Linux
  • Python version: 2.7.12-1
  • beets version: 1.3.18
  • Turning off plugins made problem go away (yes/no): No

My configuration (output of beet config) is:

paths:
    default: Music/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title
    comp: Music/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $artist - $title
    albumartist:Aerial M: Music/Pajo, David/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title
    albumartist:Papa M: Music/Pajo, David/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title
    albumartist:Pajo: Music/Pajo, David/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title
    albumtype:soundtrack: Soundtrack/Game/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $title
    albumtype:soundtrack comp: Soundtrack/Game/%the{$albumartist_sort}/$releasedate. $album%aunique{}/$disc_and_track. $artist - $title
    albumstatus:bootleg: Music/%the{$albumartist_sort}/Bootlegs/$album%aunique{}/$disc_and_track. $title
item_fields:
    disc_and_track: u'%02i' % (track) if disctotal == 1 else u'%i-%02i' % (disc, track)
    releasedate: u'%02i-%02i-%02i' % (year, month, day) if day else u'%02i-%02i' % (year, month) if month else u'%02i' % (year)
asciify_paths: yes
library: ~/.config/beets/library.db

replace:
    '[\\/]': _
    ^\.: _
    \s+$: ''
    ^\s+: ''

plugins: inline mbsubmit rewrite the
directory: ~/Media/Audio

import:
    move: yes
    write: yes
pathfields: {}
album_fields: {}
the:
    a: yes
    patterns: []
    the: yes
    strip: no
    format: '{0}, {1}'
mbsubmit:
    threshold: medium
    format: $track. $title - $artist ($length)
rewrite: {}
@sampsyo sampsyo added the bug bugs that are confirmed and actionable label Aug 10, 2016
@sampsyo sampsyo changed the title Incorrect path when containing a comma Template doesn't format at all when contains a comma Aug 10, 2016
@sampsyo
Copy link
Member

sampsyo commented Aug 10, 2016

Thanks! This looks like a nasty bug in our template parser. Here's an easy way to see what's wrong:

$ beet ls -f '$artist'
$ beet ls -f ',$artist'

Curiously, a comma following the field works fine:

$ beet ls -f '$artist,'

@diego-plan9
Copy link
Member

Following a longer than expected beets-hiatus, I was looking through the recent issues in order to try to slowly get back into active development - and stumbled upon this issue!

After looking at the syntax details docs (emphasis mine):

The characters $, %, {, }, and , are "special" in the path
template syntax. This means that, for example, if you want a % character to
appear in your paths, you'll need to be careful that you don't accidentally
write a function call. To escape any of these characters (except {), prefix
it with a $
.

I believe miblodelcarpio's problem (and the other examples on the followup comment) could be fixed by simply escaping the comma?

$ beet -vv ls -f 'Music/Pajo$, David/%the{$albumartist_sort} ...'
$ beet ls -f '$, $artist'

However, I'm wondering if it would be worth to try to improve the handling of commas in order to reduce confusion: comma (ARG_SEP) seems to be the only Parser.special_char that has a special meaning depending on specific circumstances (ie. only when used inside a group, if I'm not mistaken), and arguably the special character that is more likely to be part of a legitimate string (although it might be a stretch - I might not have solid data to backup that assumption 😏).

Would it be a good idea to revise the behaviour of the parser, trying to treat ARG_SEP as a special character only when inside a group?

@sampsyo
Copy link
Member

sampsyo commented Oct 1, 2016

Hi, @diego-plan9, and welcome back!

I agree with your analysis here 100%. You can get the comma back by escaping it, but it's likely enough to be used "incorrectly" that I think we can do better. And yes, I think the right solution is pretty much exactly that: the , character shouldn't be "special" when it appears outside of a function's argument (i.e., %f{here} in a function call).

@diego-plan9
Copy link
Member

Thanks, sampsyo!

I'll start working with that solution in mind and move the discussion to a pull request whenever a first draft is ready.

@miblodelcarpio
Copy link
Author

Oh wow, yes, this does solve my problem! Thank you, @diego-plan9.

Should we close this issue, then, since my problem is essentially fixed (user error!), or leave it open to keep track of the parser development?

@sampsyo
Copy link
Member

sampsyo commented Oct 1, 2016

Let's leave it open. Thanks for checking back in!

@diego-plan9
Copy link
Member

Closed after being fixed at #2213

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs that are confirmed and actionable
Projects
None yet
Development

No branches or pull requests

3 participants