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: replace the comma separator by new-line in ListAttribute #1626

Open
5 of 6 tasks
Exirel opened this issue May 30, 2019 · 9 comments
Open
5 of 6 tasks

config: replace the comma separator by new-line in ListAttribute #1626

Exirel opened this issue May 30, 2019 · 9 comments
Assignees
Labels
Documentation Feature Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. Medium Priority Tracking Tweak
Milestone

Comments

@Exirel
Copy link
Contributor

Exirel commented May 30, 2019

Sopel 7.x task list:

Sopel 8.x and 9.x task list:

TL;DR: in Sopel 7.x, I want to detect \n to switch to a new mode that doesn't require a delimiter with escape character.

Disclaimer: an escape mechanism was brought in for Sopel 7.x by #1460 to allow comma-separated values. I think it's a great achievement but, sadly, also something that I may want to remove entirely. I'm fully aware that this may mean having to throw off the hard work put in by @HumorBaby and I want to re-assert how much respect I've for his works, past and present.

Current state in Sopel 6.6.x

As of today's version of Sopel 6.6.x branch, this config file:

[spam]
eggs = one, two, three, four, and a half
bacons = grilled
    burn out
    greasy, fat, and tasty

will be read as this:

>>> config.spam.eggs
['one', ' two', ' three', ' four', ' and a half']
>>> config.spam.bacons
['grilled\nburn out\ngreasy', 'fat', 'and tasty']

Current state in Sopel master

In the master branch, since #1460, it is possible to escape the comma, so it would be like that:

[spam]
eggs = one, two, three, four, and a half
eggs_safe = one, four\, and a half
bacons = grilled
    burn out
    greasy, fat, and tasty

which result in eggs_safe to be properly handled, but leaving bacons still in a poor state:

>>> config.spam.eggs_safe
['one', 'four, and a half']
>>> config.spam.bacons
['grilled\nburn out\ngreasy', 'fat', 'and tasty']

And now, if we want to throw regex pattern (that need to be escaped too), what do we have?

[spam]
regex_pattern = ^Full stop\, Period\\.$, also:? this pattern$
regex_multilines =
    ^Full stop, Period\.$
    also:? this pattern$

Sure thing, regex patterns are not the easiest things to read, but I found that the multi-lines version far more readable.

What's next? What's my plan?

If we take back the example for Sopel 6.6.x:

[spam]
eggs = one, two, three, four, and a half
bacons = grilled
    burn out
    greasy, fat, and tasty

I want that output:

>>> config.spam.eggs
['one', 'two', 'three', 'four', 'and a half']
>>> config.spam.bacons
['grilled', 'burn out', 'greasy, fat, and tasty']

For Sopel 7.x I want to implement that so:

  • no escape delimiter
  • single-line expressions will be split by comma, as it is today
  • multi-line expressions will be split by break-line, *without split by comma

Then in Sopel 8.x, I want to add a "deprecation warning" on single-line expressions.

Then in Sopel 9.x, I want to remove single-line expressions handling (ie. no more split by comma).

@Exirel Exirel self-assigned this May 30, 2019
@dgw dgw added this to the 7.0.0 milestone May 30, 2019
@dgw
Copy link
Member

dgw commented May 30, 2019

OK, tagged for 7 while you get the newline stuff done (and revert whatever escaping support you need to revert). This is kind of a tracking issue, and we'll punt it to the next milestone whenever each step is done.

@HumorBaby
Copy link
Contributor

and I want to re-assert how much respect I've for his works, past and present

@Exirel you're too kind 😄 don't worry I'm not offended you want to scrap this feature so soon 😆 ya gotta do what ya gotta do... your proposed plan definitely seems like the way to go anyway 👍

@dgw
Copy link
Member

dgw commented May 30, 2019

@HumorBaby So I take it you'll be eagerly awaiting your chance to refactor #1528 on top of the newline list parsing when it's done? 😁 (Not that I know if any changes will be required, yet.)

@HumorBaby
Copy link
Contributor

So I take it you'll be eagerly awaiting your chance to refactor #1528 on top of the newline list parsing when it's done? (Not that I know if any changes will be required, yet.)

I can barely contain myself!

@HumorBaby
Copy link
Contributor

@dgw said:

So I take it you'll be eagerly awaiting your chance to refactor #1528 on top of the newline list parsing when it's done? grin (Not that I know if any changes will be required, yet.)

Only doc(+docstring) changes will be needed anywhere there is a reference to/use of comma delimiting in ListAttributes.

@dgw
Copy link
Member

dgw commented Sep 7, 2019

@Exirel After merging #1628, I have discovered that core.channels does NOT work with the new multi-line format. Channel names pretty much always start with #, which marks them as comments, so the config loads an empty option. :/

@Exirel
Copy link
Contributor Author

Exirel commented Oct 26, 2019

@dgw issue with channel has been fixed and merged into master. All is left to do for Sopel 7 is the doc part.

@dgw
Copy link
Member

dgw commented Oct 26, 2019

Doc part is done, I think. Checked off.

@dgw dgw modified the milestones: 7.0.0, 8.0.0 Oct 26, 2019
@dgw dgw added the Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. label Oct 26, 2019
@Exirel
Copy link
Contributor Author

Exirel commented Jul 23, 2022

All is done for Sopel 8.

@dgw dgw modified the milestones: 8.0.0, 9.0.0 Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Feature Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. Medium Priority Tracking Tweak
Projects
None yet
Development

No branches or pull requests

3 participants