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

List dumping should count total chars #119

Open
iamkroot opened this issue Oct 31, 2020 · 1 comment
Open

List dumping should count total chars #119

iamkroot opened this issue Oct 31, 2020 · 1 comment

Comments

@iamkroot
Copy link

The current version counts the number of elements in the list to determine whether it should be inline or not.

length = len(data)
if self.default_flow_style is None and length < 4:
node.flow_style = True
elif self.default_flow_style is None:

This works fine when the representation of the elements themselves are pretty short, but if we're storing large(-ish) strings, things get out of hand, even when the number of elements is less than 4. Here's an example for a config file I use to store user-defined regexes:

    include_regexes:
        episode: ['.*?/(?P<title>[^/]+?)(\s+\[[0-9]{3,4}p\])?(/S(eason)?\s*(?P<season>\d+))?/(?P<episode>\d+) - (?:.*)', '.*?/(?P<title>[^/]+?)(\s+\[[0-9]{3,4}p\])?(/S(eason)?\s*\d+)?/S(?P<season>\d+)E(?P<episode>\d+) - (?:.*)']
        movie: ['.*/(?P<title>.+?) \((?P<year>[0-9]{4})\)[^/]+$']

Possible solutions

  1. The best case scenario would be to have some way to get the total length of the elements after dump, and have a threshold on the number of chars to decide. But from my brief look over the pyyaml api, this doesn't seem to be feasible.
  2. As a workaround, I'd be ok if the Configuration.dump() method exposes the default_flow_style parameter as a keyword argument, so that we can enforce no inlining of lists.
@sampsyo
Copy link
Member

sampsyo commented Oct 31, 2020

Seems like a reasonable thing to want! FWIW, the "real solution" may be #52, i.e., switching to a YAML library that has reasonable round-trip dumping built in so we don't need to resort to these hacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants