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

config: FilenameAttribute doesn't handle quotes well #2263

Closed
dgw opened this issue Mar 1, 2022 · 4 comments · Fixed by #2371
Closed

config: FilenameAttribute doesn't handle quotes well #2263

dgw opened this issue Mar 1, 2022 · 4 comments · Fixed by #2371
Labels
Bug Things to squish; generally used for issues
Milestone

Comments

@dgw
Copy link
Member

dgw commented Mar 1, 2022

Description

If the value of a FilenameAttribute set in the config file is quoted, it silently creates a mess.

Reproduction steps

  1. Set core.logdir = "/path/to/logs"
  2. Run Sopel
  3. Sopel quietly creates a silly directory tree inside $HOMEDIR:
$ tree ~/.sopel
/home/dgw/.sopel
├── "
│   └── path
│       └── to
│           └── logs"
│               ├── default.error.log
│               ├── default.exceptions.log
│               ├── default.raw.log
│               └── default.sopel.log
...

Expected behavior

Even though quotes aren't required in Sopel's config file, they should be sensibly handled.

Environment

  • Sopel .version: current master, aca1055
  • Sopel installed via: -e . @ above commit
  • Python version: 3.8, 3.9
  • Operating system: Ubuntu 20.04 (WSL)

Notes

This is a leftover todo from #1959, where I updated the docs but didn't make any code changes related to this.

@dgw dgw added the Bug Things to squish; generally used for issues label Mar 1, 2022
@dgw dgw added this to the 8.0.0 milestone Mar 1, 2022
@half-duplex
Copy link
Member

Can we just switch to TOML >.>

@dgw
Copy link
Member Author

dgw commented Mar 1, 2022

Definitely not for 8.0.

@Exirel
Copy link
Contributor

Exirel commented Mar 1, 2022

As written in the doc:

Quoting a value unnecessarily can lead to unexpected behavior such as an absolute pathname being interpreted as relative to Sopel’s home directory.

@dgw
Copy link
Member Author

dgw commented Mar 1, 2022

Yes @Exirel, that's what I added in #1959. But in the interest of user-friendliness (Sopel is supposed to be "simple", remember) I think we should add sensible behavior like stripping quotes (and logging a warning), unless there are reasonable use cases where someone might intentionally want a folder named ".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants