Skip to content

docs: add example to count ERROR lines in a log file (#236)#243

Open
shreyaabaranwal wants to merge 3 commits into
bitfield:masterfrom
shreyaabaranwal:add-examples
Open

docs: add example to count ERROR lines in a log file (#236)#243
shreyaabaranwal wants to merge 3 commits into
bitfield:masterfrom
shreyaabaranwal:add-examples

Conversation

@shreyaabaranwal

Copy link
Copy Markdown

Hi! Following up on #236 — here's a first example to get the examples/ directory started.

I went with something straight out of everyday ops life: counting ERROR lines in a log file. Anyone who's stared at logs during an incident knows this one — it's basically grep ERROR app.log | wc -l, but here it's done with script:

script.File(logFile).Match("ERROR").CountLines()

I really like how cleanly the pipe-style API reads here — it maps almost one-to-one to the shell command, which I think makes it a nice, relatable first example for newcomers.

I've included a small sample examples/app.log so it runs out of the box, and verified everything locally with go run, gofmt -l, and go vet (all clean).

Happy to add a couple more examples in the same spirit if you'd like — just wanted to start with one and get your feedback on the format first.

Closes #236

Signed-off-by: shreyaabaranwal <shreyabaranwal229@gmail.com>
Signed-off-by: shreyaabaranwal <shreyabaranwal229@gmail.com>
@bitfield

Copy link
Copy Markdown
Owner

Great contribution, @shreyaabaranwal, thanks a lot!

It might be a nice idea to tie these examples in more directly with the ones in the README, mightn't it? For example, we start with:

contents, err := script.File("test.txt").String()

and work our way through things like counting error lines (to your point). What if we made each of these examples a standalone program, and then we could link from the README to the full listing? If you feel like adding more complicated examples, by all means do, but we should probably start with these.

For examples that need to read a specific file, we should probably come up with a general solution to:

	// Check if examples/app.log exists, otherwise fallback to app.log.

My instinct is that people would naturally run go run examples/count_errors.go, for instance, which would mean the path we're opening should be examples/app.log. We want to minimise any code in the example that's not directly about script, purely because that makes it easier for people to see what the example is actually about.

…itfield#236)

Signed-off-by: shreyaabaranwal <shreyabaranwal229@gmail.com>
@shreyaabaranwal

Copy link
Copy Markdown
Author

Thanks for the feedback! Reworked both examples:

  • Dropped the file-existence fallback — they now open examples/app.log (and examples/servers.csv) directly, since people run them from the repo root with go run examples/count_errors.go.
  • Stripped out everything that wasn't about script itself, so the pipeline line stands on its own.
  • Linked them from the README right after the matching "count Error lines" snippet, following the existing progression.

Verified locally with go run, gofmt -l, and go vet (all clean). Happy to add more in the same style once the format looks right.

@bitfield bitfield left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Amazing work @shreyaabaranwal, thanks! This is super helpful.

I've suggested a couple of presentational tweaks, but otherwise I think this is ready to merge as is. Nice job.

Comment thread examples/count_errors.go
@@ -0,0 +1,26 @@
//go:build ignore

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm wondering if we actually need this. I mean, it's absolutely correct in the sense that this file isn't part of the script package. On the other hand, one thing about example programs is that people will copy and paste them exactly as is—that's what they're for, after all. Including this line would stop their program working, which might be very puzzling for beginners.

Does it do us any harm to omit the build tag in examples?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good question — I tried it, and removing the build tag does break things: with both count_errors.go and filter_csv.go declaring func main() in the same examples/ directory, go build ./... and go vet ./... fail with "main redeclared". go run file.go handles one file fine, but a directory-wide build compiles them as one package, so the tag was working around exactly that.

To keep them copy-paste-able without the tag, my instinct is to give each example its own subdirectory (examples/count_errors/main.go, examples/filter_csv/main.go) — scales cleanly and go build ./... stays happy. But it's your call on the layout. Which would you prefer?

Comment thread examples/count_errors.go
func main() {
count, err := script.File("examples/app.log").Match("ERROR").CountLines()
if err != nil {
panic(err)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Again, panicking on errors makes sense for an example because we don't want to clutter up the program with error handling paperwork. But, again, I've learned over the years that whatever you show people is exactly what they'll do, word for word. It's no good saying “Obviously for real programs you wouldn't use panic here,” because that's not obvious to beginners, and they are precisely the people we're writing for.

Instead, even when it's not directly relevant to the principle I'm demonstrating, I've got into the habit of writing the code that shows what I think is best practice:

Suggested change
panic(err)
fmt.Fprintln(os.Stderr, err)
os.Exit(1)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will do — switching to fmt.Fprintln(os.Stderr, err) + os.Exit(1) when I apply the restructure.

Comment thread README.md
numErrors, err := script.File("test.txt").Match("Error").CountLines()
```

You can find a complete, runnable version of this in [examples/count_errors.go](examples/count_errors.go), along with other examples in the [`examples/`](examples) directory.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

In my books I use a listing link convention something like this:

Suggested change
You can find a complete, runnable version of this in [examples/count_errors.go](examples/count_errors.go), along with other examples in the [`examples/`](examples) directory.
(Listing [`count_errors`](examples/count_errors.go))

We can use that style for all the other linked examples to come, and perhaps this first time mention the examples directory:

Suggested change
You can find a complete, runnable version of this in [examples/count_errors.go](examples/count_errors.go), along with other examples in the [`examples/`](examples) directory.
You can find a complete, runnable version of this and other examples in the [`examples/`](examples) directory.

What do you think?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I like the (Listing name) convention — I'll use the directory-mention version for this first link and that style for the rest, once we settle the layout.

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.

Add examples/ in the repo

2 participants