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

Example implementation for the filters #282

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dev-ardi
Copy link
Contributor

@dev-ardi dev-ardi commented Nov 15, 2023

There's still a lot to do and think. This is just a draft to have an example to think about and work on.

This just is a list of composable filters that fit into another. This is really extensible, but there are a few things left to think.

There are many optimizations possible with this model.

I don't like having possible failures inside of the filters, everything should have been figured out by this time, but I've implemented it just in case

This was referenced Nov 15, 2023
@CosmicHorrorDev
Copy link
Collaborator

I'll look at this more after #265 gets merged since I'm assuming that will change at least some of this PR

@dev-ardi
Copy link
Contributor Author

@CosmicHorrorDev

will change at least some of this PR

Not really. This is atomic and only the "filter" part. There's still so much to do.

@CosmicHorrorDev
Copy link
Collaborator

I can take a look after I'm no longer sick

@dev-ardi
Copy link
Contributor Author

Get better soon!

@CosmicHorrorDev
Copy link
Collaborator

Sorry for taking so long to give feedback

I think the main fundamental thing that will cause problems and have to be addressed is how to handle streaming in this model. Like in the case of something like

$ sd --pretend-we-have-a-line-mode 'bar' 'baz'
foo
bar
baz <-- output by `sd`

we need to be able to stream the output line by line which I think means that we'll need to have an api that looks more like the std::io apis where you dealing more with reading and writing and different layers can allocate as needed

Some various other thoughts:

  • The filter operations have to be fallible since we won't be able to resolve everything ahead of time. You already talked about how this is supported, but I just wanted to note that this a hard requirement
  • The fundamental type here should be bytes instead of utf8. We want to support being able to do byte operations for people and we want to be able to support different encodings as well
    • That means that utf8 or any other kind of text decoding will have to be done on the fly instead of entirely up-front
    • --preview is a special case where we want to only output utf8, so that could be represented as an extra Filter applied at the end

All that out of the way that doesn't mean we need something that can fit all of those needs now, but I think it does limit how Filter can be designed

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.

None yet

2 participants