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.

Sign up for GitHub

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

Jump to bottom

Set row_sep=None by default and write a '\n' after row_sep (when it is provided) #41

Closed
janluke opened this issue Jun 15, 2021 · 0 comments · Fixed by #42
Closed

Set row_sep=None by default and write a '\n' after row_sep (when it is provided) #41

janluke opened this issue Jun 15, 2021 · 0 comments · Fixed by #42
Labels
breaking change This breaks backward compatibility
Milestone
v0.9.0

Comments

@janluke
Copy link
Owner

janluke commented Jun 15, 2021

Motivation

  • The use of "" to indicate "no value" makes type checking less useful. Using None is more correct.
  • A separator not ending with \n doesn't make any sense. So the \n should be be enforced in some way.
  • I don't want to require SepGenerator to return a string ending with \n. If it needs to be there, why should I bother the user to add it? Furthermore, if one wants to use Hline in other parts of the program, s(he) may need to rstrip() the returned separator returned by Hline, which is not ideal.

Decision

  • Set row_sep=None by default.
  • Make the formatter add the \n after the row_sep string (when row_sep is not None). In other words, do not bother the user to include a '\n' at the end of row_sep.

This is a breaking change.
@janluke janluke added the breaking change This breaks backward compatibility label Jun 15, 2021
@janluke janluke added this to the v0.9.0 milestone Jun 15, 2021
janluke added a commit that referenced this issue Jun 16, 2021
@janluke
Redesign row_sep
99767fe 
1. Set it to `None` by default and write a `\n` after it (closes #41).
2. Insert `row_sep` only in between rows (closes #36).
3. Define `SepGenerator` as a protocol and allow to pass a generator
   to `row_sep` (closes #39).
4. Implements the `Hline` SepGenerator.
5. Define a `RowSepPolicy` base class and implement it with `RowSepIf`
   (closes #37).
@janluke janluke mentioned this issue Jun 16, 2021
Merged
janluke added a commit that referenced this issue Jun 16, 2021
@janluke
Redesign row_sep (#42)
a519ec1 
Redesign row_sep

1. Set it to `None` by default and write a `\n` after it (closes #41).
2. Insert `row_sep` only in between rows (closes #36).
3. Define `SepGenerator` as a protocol and allow to pass a generator
   to `row_sep` (closes #39).
4. Implements the `Hline` SepGenerator.
5. Define a `RowSepPolicy` base class and implement it with `RowSepIf`
   (closes #37).
@janluke janluke mentioned this issue Jun 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking change This breaks backward compatibility
Projects
None yet
Milestone
v0.9.0
Development

Successfully merging a pull request may close this issue.

Redesign row_sep janluke/cloup
1 participant
@janluke