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

New function table.AppendRowLine() to manually add row separators. #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mittwingate
Copy link

Here's a quick addition for manually adding line separators, hope you like it!

@mattn
Copy link
Collaborator

mattn commented Apr 9, 2019

Please add test.

@mittwingate
Copy link
Author

Actually, the test is already included in the PR, it's the "Output 7" part that lists the expected result which gets created on "go test" (Example function).

@olekukonko
Copy link
Owner

@mattn this has been opened for a while now, what do you think?

table.go Outdated
@@ -274,6 +277,11 @@ func (t *Table) SetBorders(border Border) {
t.borders = border
}

// Append a separator after the current line
func (t *Table) AppendRowLine() {
t.rowLineIdxs[len(t.lines)-1]++
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the lines is empty, how this work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it won't do any harm because it's a mapping that's later ignored (and rowLineIdxs was a map[int]int, not an array), but still worth fixing. What I found while adding more tests, though, is that there's two different row printing methods, printRows() and printRowsMergeCells(), which both need to support the new manually added row separators, I'll update my branch with the changes.

@mittwingate
Copy link
Author

Okay, I've updated the PR with new test cases and support for both printRows() and printRowsMergeCells(), please take a look and let me know if you like it!

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

Successfully merging this pull request may close these issues.

3 participants