Repository: beetbox/beets
Pull request: Add documentation for `path_sep_replace`
## Description

Fixes https://github.com/beetbox/beets/issues/323 (probably).

This PR adds documentation for `path_sep_replace` configuration option, which is not yet present in the official docs.

## To Do

- [x] Documentation. (If you've add a new command-line flag, for example, find the appropriate page under `docs/` to describe it.)
- [ ] Changelog. (Add an entry to `docs/changelog.rst` near the top of the document.)
- [ ] Tests. (Encouraged but not strictly required.)


Problem statement:
Document or remove path_sep_replace config value
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.

Hints from issue discussion:
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.

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?

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.

So, this feature is apparently still implemented, [it](https://github.com/beetbox/beets/blob/master/beets/dbcore/db.py) has the relevant parts still there:

``` python
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?

@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).

@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.

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

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
> https://github.com/beetbox/beets/issues/323#issuecomment-232137938, or mute
> the thread
> https://github.com/notifications/unsubscribe/AHa0XPLAIxO7JlITbJ8ZgXwZ5jFwEKXCks5qU935gaJpZM4AuA9x
> .

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](https://github.com/beetbox/beets/files/360201/beets-logfile.txt)

[config.yaml.txt](https://github.com/beetbox/beets/files/360205/config.yaml.txt)

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.

Commit message: Update config.rst

This PR adds documentation for `path_sep_replace` configuration option, which is not yet present in the official docs.

Patch to review:
diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index f041be6250..636a018ede 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -151,6 +151,19 @@ replaced as they don't match the typewriter quote (``"``). To also strip these
 special characters, you can either add them to the replacement list or use the
 :ref:`asciify-paths` configuration option below.
 
+.. _path-sep-replace:
+
+path_sep_replace
+~~~~~~~~~~~~~~~~
+
+A value that replaces the path separator in the filenames created by beets.
+On most systems, the path separator is the forward slash (``/``).
+
+Note that you should never use the actual path separator as the value for this option,
+otherwise it's very easy to mess up the structure of your music directory.
+
+Default: `_`.
+
 .. _asciify-paths:
 
 asciify_paths


Code review comments:
diff_hunk: @@ -151,6 +151,19 @@ replaced as they don't match the typewriter quote (``"``). To also strip these
 special characters, you can either add them to the replacement list or use the
 :ref:`asciify-paths` configuration option below.
 
+.. _path-sep-replace:
+
+path_sep_replace
+~~~~~~~~~~~~~~~~
+
+A value that replaces the path separator in the filenames created by beets.
+On most systems, the path separator is the forward slash (``/``).
+
+Note that you should never use the actual path separator as the value for this option,
+otherwise it's very easy to mess up the structure of your music directory.
+
+Default: `_`.
path: docs/reference/config.rst
text: @user1:
```suggestion
Default: ``_``.
```

That's the ReST syntax for code.