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

Add ability to intercept streams with middleware #93

Merged
merged 3 commits into from
Oct 27, 2024

Conversation

mgazza
Copy link
Contributor

@mgazza mgazza commented Aug 12, 2024

Add the ability to add support for such things as zmodem and/or other
stream middleware's

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

I know this is a common concept in HTTP handling, though the implementation doesn't seem to quite match by introducing the requirement to implement types instead of just a function?
Also is the "SetMiddleware" a clear enough name? In the realm of terminals or GUIs I'm not sure it`s totally clear...

term.go Outdated
@@ -497,3 +504,20 @@ func (t *Terminal) Dragged(d *fyne.DragEvent) {
func (t *Terminal) DragEnd() {
t.selecting = false
}

// SetMiddleware sets the middleware function that will be used when creating a new terminal.
// The middleware function is responsible for setting up the I/O readers and writers.
Copy link
Member

Choose a reason for hiding this comment

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

The way middleware is implemented it is an interface type not function.
It feels like a function would be enough to set up the interception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats the beauty of golang right?

// MiddlewareFunc is a function type that matches the signature of the
// SetupMiddleware method in the Middleware interface.
type MiddlewareFunc func(r io.Reader, w io.WriteCloser) (io.Reader, io.WriteCloser)

// SetupMiddleware allows MiddlewareFunc to satisfy the Middleware interface.
// It calls the MiddlewareFunc itself.
func (m MiddlewareFunc) SetupMiddleware(r io.Reader, w io.WriteCloser) (io.Reader, io.WriteCloser) {
	return m(r, w)
}

Copy link
Member

Choose a reason for hiding this comment

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

I guess if following the http middleware / handler idea it was missing the func helper? https://pkg.go.dev/net/http#HandlerFunc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes essentially the same pattern

@andydotxyz
Copy link
Member

Sorry for the test failures, fixed on master

Add the ability to add support for such things as zmodem and/or other
stream middleware's
@mgazza
Copy link
Contributor Author

mgazza commented Aug 14, 2024

Sorry for the test failures, fixed on master

no worries rebased

@andydotxyz
Copy link
Member

Also is the "SetMiddleware" a clear enough name? In the realm of terminals or GUIs I'm not sure it`s totally clear...

I think this is still niggling me - it's such a generic term that I have never heard use in stream handling or terminals before.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks, I think that is clear now

@andydotxyz andydotxyz merged commit bc7d6e3 into fyne-io:master Oct 27, 2024
5 checks 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.

2 participants