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

Document or remove path_sep_replace config value #323

Open
mrjoel opened this issue Jun 8, 2013 · 13 comments

Comments

@mrjoel
Copy link

commented Jun 8, 2013

I struggled for quite a while to not have slashes replaced with underscores (I use a comma for readability). Based on the documentation of the configuration (which is otherwise quite thorough, thanks!), I tried replacing the regex in the replace configuration value. Only after digging at the code did I find that there is a separate path_sep_replace config value used to define what to use for replacement.

I didn't look in detail, but it looks like as long as a valid regex replacement value is given then the separate config value isn't needed. If it is needed, a quick mention during the discussion of the replace configuration section would be quite helpful.

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jun 8, 2013

You're right: we should document this better. I've been surprised by the number of people who have gone looking for this setting.

There are two reasons that the setting is currently separate: (a) so we can use os.path to determine what the platform's path separators are in a hypothetical universe where there's something other than / and , and (b) so that a user can less easily shoot themselves in the foot by accidentally removing the path separator replacement, which could lead to all manner of unpleasantness. I'm not entirely sure if either of these reasons is valid anymore.

sampsyo referenced this issue Feb 13, 2014

sampsyo referenced this issue Feb 22, 2014

Revert "Replace path separators from config"
This reverts commit c82b31e.

Conflicts:
	docs/changelog.rst

@brilnius brilnius removed the bitesize label Mar 18, 2014

@pnelson

This comment has been minimized.

Copy link

commented Apr 1, 2015

I ran into this one today. It has been a while since the original report or references. Have you given any more thought on this?

@sampsyo

This comment has been minimized.

Copy link
Member

commented Apr 1, 2015

I do think about it from time to time, but I'm still convinced there's no good solution: the current behavior is confusing, and the ideal behavior is surprisingly hard to get right. Bright ideas are always welcome.

@petwri

This comment has been minimized.

Copy link

commented May 8, 2016

So, this feature is apparently still implemented, it has the relevant parts still there:

if self.for_path:
    sep_repl = beets.config['path_sep_replace'].get(unicode)
    for sep in (os.path.sep, os.path.altsep):
        if sep:
            value = value.replace(sep, sep_repl)

It should replace any occurence of "/" on my linux system with the 'path_sep_replace' from my config. But, for some reason, it is not. Any ideas?

@sampsyo

This comment has been minimized.

Copy link
Member

commented May 8, 2016

@petwri We might be able to help, but we'll need more details (i.e., the standard bug report stuff including a specific way to reproduce the problem).

@petwri

This comment has been minimized.

Copy link

commented Jul 12, 2016

@sampsyo Sorry I am coming back to this after not paying any attention to it for quite a while, I just found out that it is still in the code (see my posting from 8 May). But I can't really tell why "sep" doesn't get replaced by "sep_rep1". If you need, I could provide more info.

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jul 12, 2016

Yes, please provide a full bug report. A new ticket is probably in order.

@petwri

This comment has been minimized.

Copy link

commented Jul 12, 2016

Will asap.

On Jul 12, 2016 20:33, "Adrian Sampson" notifications@github.com wrote:

Yes, please provide a full bug report. A new ticket is probably in order.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#323 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AHa0XPLAIxO7JlITbJ8ZgXwZ5jFwEKXCks5qU935gaJpZM4AuA9x
.

@petwri

This comment has been minimized.

Copy link

commented Jul 12, 2016

So, tried it again, importing and moving resulted in this:

user@system:~/.scripts$ beet ls -f '$path'
** plugin echonest not found
/media/Music/Steven Wilson/4 1/2/01-01.mp3
/media/Music/Steven Wilson/4 1/2/01-02.mp3
/media/Music/Steven Wilson/4 1/2/01-03.mp3
/media/Music/Steven Wilson/4 1/2/01-04.mp3
/media/Music/Steven Wilson/4 1/2/01-05.mp3
/media/Music/Steven Wilson/4 1/2/01-06.mp3

So, the / in 1/2 is still there, I expected it to get replaced with a _

Version is 1.3.18, Python 2.7.11+, Linux 4.5.2-040502-generic

beets-logfile.txt

config.yaml.txt

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jul 12, 2016

Huh, that's odd. This is a separate issue, though—can you please open a new bug so we don't bother the subscribers to this issue?

Then, it might be useful to see if this happens in the default configuration, with everything turned off. Out of the box, beets should replace slashes automatically.

@petwri

This comment has been minimized.

Copy link

commented Jul 13, 2016

When you're talking about default config, where do I get that from?

On Wed, Jul 13, 2016 at 12:01 AM, Adrian Sampson notifications@github.com
wrote:

Huh, that's odd. This is a separate issue, though—can you please open a
new bug so we don't bother the subscribers to this issue?

Then, it might be useful to see if this happens in the default
configuration, with everything turned off. Out of the box, beets should
replace slashes automatically.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#323 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AHa0XLQmShFRigQuwnKsb61iytPn_HKZks5qVA6rgaJpZM4AuA9x
.

@jackwilsdon

This comment has been minimized.

Copy link
Member

commented Jul 13, 2016

You can just (temporarily) move your config.yaml elsewhere (e.g. renaming it) which should set the configuration back to default. 😄

I'd take a backup of my whole beets directory too if I were you. 😆

@petwri

This comment has been minimized.

Copy link

commented Jul 13, 2016

Issue created: #2126

meribold added a commit to meribold/dotfiles that referenced this issue Jul 23, 2019

Beets: fix `/` in tags not mapping to `-` in paths
Apparently we need to use the secret `path_sep_replace` option.  See
<beetbox/beets#323>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.