Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat(local): sg tail #64146

Merged
jhchabran merged 11 commits into
mainfrom
jh/sgtail
Jul 30, 2024
Merged

feat(local): sg tail #64146
jhchabran merged 11 commits into
mainfrom
jh/sgtail

Conversation

@jhchabran

@jhchabran jhchabran commented Jul 30, 2024

Copy link
Copy Markdown
Contributor

This PR brings back https://github.com/sourcegraph/sgtail back in sg, plus a few adjustments to make it easier to use. I'll archive that repo once this PR lands.

@camdencheek mentioned you here as you've been the most recent beta tester, it's more an FYI than a request for a review, though it's welcome if you want to spend a bit of time reading this.

Closes DINF-155

Test plan

Locally tested + new unit test + CI

Changelog

  • Adds a new sg tail command that provides a better UI to tail and filter log messages from sg start --tail.

@cla-bot cla-bot Bot added the cla-signed label Jul 30, 2024
@jhchabran jhchabran requested review from a team and camdencheek July 30, 2024 09:45
Comment thread dev/sg/sg_start.go Outdated
Comment thread dev/sg/tail/activity.go Outdated
Comment thread dev/sg/tail/activity_test.go Outdated
Comment thread dev/sg/tail/model.go Outdated
Comment thread dev/sg/tail/model.go
m.refreshContent()
case "tabclose":
if m.tabIndex == 0 {
// TODO print something

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think bubbletea has toast support but would be nice to print "can't close the main tab" or something like that

Comment thread dev/sg/tail/model.go Outdated
}, nil
case "grep":
if len(parts[1:]) < 1 {
return nil, errors.Newf("drop requires at least one arguments")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, errors.Newf("drop requires at least one arguments")
return nil, errors.Newf("grep requires at least one argument")

Comment thread dev/sg/tail/model.go
return ""
}

func max(a, b int) int {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

golang has builtin max now :D

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.

Yeah but it's f64, so I'd rather do this than dealing with the conversion ^^

Comment thread dev/sg/tail/socket.go Outdated
Comment thread dev/sg/tail/model.go Outdated
func (m model) Init() tea.Cmd {
return tea.Batch(
showUsage(),
listenUnixSocket(m.l, m.ch),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This panics when it can't connect. Should it not rather post activityMsg with the error that it could not connect?

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're on the listener side, so if we can't even create the unix socket for others to connect onto it, we'd better quit right away. I'll check if at least I can make it not panicking though.

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.

Ah, need to make it possible to bubble errors from a tea.Cmd, I'll do that on another iteration, I don't want to spend more time on this today.

@burmudar burmudar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing really blocking 🙌🏼

@jhchabran jhchabran enabled auto-merge (squash) July 30, 2024 11:41
@jhchabran jhchabran disabled auto-merge July 30, 2024 11:50
@jhchabran jhchabran merged commit bc4acd1 into main Jul 30, 2024
@jhchabran jhchabran deleted the jh/sgtail branch July 30, 2024 12:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants