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

feat: tailspin as a library #116

Closed
wants to merge 3 commits into from
Closed

Conversation

destifo
Copy link

@destifo destifo commented Feb 2, 2024

to use tailspin as a library in addition to binary crate.

@bensadeh
Copy link
Owner

bensadeh commented Feb 5, 2024

Hi @destifo and thanks for opening this PR!

This is a use case I have not thought about, but it makes a lot of sense seeing the use case in the zifeo/whiz#117 PR for example.

In general, I think that opening up tailspin as a library (in addition to the binary) is a good idea and something that I'd like to explore in the future.

However, tailspin in its current state is not ready to be opened up as a library because it is not designed that way from the beginning, and all of the internals are exposed. As a library, we would expose only functions for highlighting etc.

To sum up: I think this direction makes a lot of sense, but it requires some larger restructuring of the code in order to expose the highlighting functionality only. Additionally/alternatively, we could extract the highlighting functions to its own repo and completely decouple it from the tailspin binary.

@destifo
Copy link
Author

destifo commented Feb 5, 2024

Thanks for reaching back! Yeah, decoupling only the highlighting feature makes a lot of sense as there are some parts which are not needed in the library crate such as reading arguments from the terminal. I will try to look into ways I can contribute as well.

@zifeo
Copy link

zifeo commented Feb 5, 2024

@bensadeh thanks for commenting in that direction. Do you see a potential short-term middleground between this PR and the global effort you are suggesting (which I understand would require more time)?

@bensadeh
Copy link
Owner

bensadeh commented Feb 6, 2024

Hello @zifeo, I appreciate your input.

Unfortunately, finding a middle ground between this PR and transitioning to a library in the near term seems unfeasible. This shift demands significant reorganization of the code and careful consideration of the interface of the exposed functions. While evolving tailspin in this direction seems like the logical progression, it's a substantial undertaking.

Implementing this PR as it stands would inadvertently position tailspin as a ready-to-use library, which is concerning. This change suggests readiness for library use, potentially leading to frequent breaking changes due to the high visibility of internal mechanics. This scenario is likely to introduce more complications and increase the maintenance burden.

@zifeo
Copy link

zifeo commented Feb 6, 2024

@bensadeh Would you accept to merge this against a parallel branch in your repo? That could satisfy all sides.

@bensadeh
Copy link
Owner

bensadeh commented Feb 7, 2024

In my opinion, these changes are best suited to be placed in a fork of tailspin, like @destifo's one here: https://github.com/destifo/tailspin.

I understand that this is not ideal for you, but tailspin is a passion project I work on on my spare time, and I am trying to reduce overhead and increase maintainability for myself as much as possible. 🙏

@bensadeh
Copy link
Owner

bensadeh commented Feb 7, 2024

In the meantime, I created #118 to hold the discussion / direction of how a tailspin create would function.

@bensadeh bensadeh closed this Feb 9, 2024
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