Skip to content

fix issue with report dealing newlines, also turned lib to go module #23

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marcocarpani
Copy link

fixed isue #19
also adding support for GO module

@arp242
Copy link

arp242 commented Sep 8, 2021

Thanks! Would you mind adding a test case in TestParseReport() over here as well?

spamc.go Outdated
last := len(report.Table) - indexShift
if last >= 0 {
line = strings.TrimSpace(line)
report.Table[last].Description += "\n " + line
Copy link

Choose a reason for hiding this comment

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

I think this would be clearer with strings.Repeat(); for example (untested):

Suggested change
report.Table[last].Description += "\n " + line
report.Table[last].Description += "\n" + strings.Repeat(" ", 28) + strings.TrimSpace(line)

spamc.go Outdated
} else {
indexShift := 1

last := len(report.Table) - indexShift
Copy link

Choose a reason for hiding this comment

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

Since indexShift is always 1 you can subtract it directly:

Suggested change
last := len(report.Table) - indexShift
last := len(report.Table) - 1 // Maybe comment why - 1?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, agreed with everything. Later on, i'll do test and any code modification as suggested. Thanks 👍

@arp242
Copy link

arp242 commented Sep 11, 2021

@ripexz Could you merge this please? Or give me access to this repo?

@ripexz
Copy link
Member

ripexz commented Sep 13, 2021

Would be better to keep the move to go.mod in a separate PR but since it's here already - please update the build script as its running on pre-gomod go versions so it's failing. Also some lint errors in there it seems, but those might also be related to go version 🤔

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