Skip to content

Add number-lines processor#65

Merged
abhimanyu003 merged 8 commits intoabhimanyu003:mainfrom
kamchy:numberlines
Sep 12, 2025
Merged

Add number-lines processor#65
abhimanyu003 merged 8 commits intoabhimanyu003:mainfrom
kamchy:numberlines

Conversation

@kamchy
Copy link
Copy Markdown
Contributor

@kamchy kamchy commented Aug 30, 2025

number-lines (alias: nl) prepends consecutive numbers to non-empty lines.

kamchy added 2 commits August 30, 2025 07:15
numberlines prepends non-empty lines with consecutive numbers
Copy link
Copy Markdown
Collaborator

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

Good addition, some questions and nitpicking anyway

}

func (p LineNumberer) Alias() []string {
return []string{"nl", "number-lines"}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is no need to add the name as an alias

Suggested change
return []string{"nl", "number-lines"}
return []string{"nl"}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 090ccbd

type LineNumberer struct{}

func (p LineNumberer) Name() string {
return "number-lines"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm unsure about the name here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So am I. Initially I wanted "line-numbers" and the matching alias "ln" would suggest creating links. And "nl" alias mimicks the "nl" binary from coreutils, so I went this path...

result += line
} else {
result += fmt.Sprintf("%d. %s", counter, line)
counter += 1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
counter += 1
counter++

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 652c4c6

Comment on lines +25 to +26
if line == "\n" {
result += line
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is this case?

When does it happen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We keep empty lines on output but we don't want to number them.

}

func (p LineNumberer) Transform(data []byte, _ ...Flag) (string, error) {
var s = string(data[:])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
var s = string(data[:])
var s = string(data)

This, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, slicing is redundant. Fix: c91096d

}

}
return result, err
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't add an err variable, simply do this

Suggested change
return result, err
return result, nil

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fix in 652c4c6

if line == "\n" {
result += line
} else {
result += fmt.Sprintf("%d. %s", counter, line)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We may consider to pad with space

Because

1. foo
…
9. foo
10. too

could be this

1.  foo
…
9.  foo
10. too

Or this

 1. foo
…
 9. foo
10. too

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Evil is in detail

Copy link
Copy Markdown
Contributor Author

@kamchy kamchy Aug 30, 2025

Choose a reason for hiding this comment

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

I try left-pad (last suggestion above) in 8c8c68e.

@abhimanyu003
Copy link
Copy Markdown
Owner

Hello @kamchy many thanks for adding this new feature 👏 💯 🥳
Also thanks to @ccoVeille for reviewing the PR. 🙇

Whats next: I will check and merge it over this week and do a new release.

@abhimanyu003 abhimanyu003 merged commit 459c5a8 into abhimanyu003:main Sep 12, 2025
1 check passed
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