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: patterns highlighting #117

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: patterns highlighting #117

wants to merge 11 commits into from

Conversation

destifo
Copy link
Contributor

@destifo destifo commented Jan 31, 2024

The highlighting is done by the tailspin crate.

Screenshot 2024-01-31 at 12 53 45 PM Screenshot 2024-01-31 at 12 53 56 PM Screenshot 2024-01-31 at 12 54 03 PM Screenshot 2024-01-31 at 12 54 08 PM Screenshot 2024-01-31 at 12 54 12 PM Screenshot 2024-01-31 at 12 54 15 PM Screenshot 2024-01-31 at 12 54 19 PM Screenshot 2024-01-31 at 12 54 23 PM Screenshot 2024-01-31 at 12 54 27 PM

@destifo destifo marked this pull request as draft January 31, 2024 15:10
@destifo destifo marked this pull request as ready for review February 1, 2024 19:33
src/actors/command.rs Outdated Show resolved Hide resolved
Copy link
Owner

@zifeo zifeo left a comment

Choose a reason for hiding this comment

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

Lgtm, can you link the pull request upstream (we will wait on the maintainer answer before merging)?

cli::{self, Cli},
highlight_processor::HighlightProcessor,
highlighters,
theme::{self, processed::Theme},
Copy link
Owner

Choose a reason for hiding this comment

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

we usually keep all files in the actors folder as actor, maybe this file can be moved elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that makes sense. I will move it to the utils module and update my branch.

@destifo
Copy link
Contributor Author

destifo commented Feb 3, 2024

Yeah, I will send the PR to the tailspin repo and wait reply from the maintainers.

@destifo
Copy link
Contributor Author

destifo commented Feb 3, 2024

I have raised the PR for the tailspin repo here.

@destifo
Copy link
Contributor Author

destifo commented Feb 4, 2024

I have moved the code to utils file, but I think it's better to move the utils.rs to a new folder and place the highlighter code to separate file which will be under the utils module. I didn't do that cause I didn't wanna change that much of the codebase, but if it's ok, I will.

@zifeo
Copy link
Owner

zifeo commented Feb 4, 2024

@destifo go 👍

@destifo
Copy link
Contributor Author

destifo commented Feb 5, 2024

I have reorganized the util files under one directory. We will only modify the custom_highlighter when there are changes related to the highlighting in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants