refact: rust fixes and optimizations

[jeevithakannan2]

Thanks @lj3954 and @cartercanedy for your reviews and suggestions. Learnt a lot from you guys 😄

Type of Change

• Bug fix
• Refactoring
• UI/UX improvement

Notable changes so for

• 1aa1dfe Use specified theme color instead of using ratatui::Color for running_command.rs success and fail color + filter.rs preview text color + state.rs min-tui warning color, confirmation.rs add colors for confirmation prompt, theme.rs fix inverted success and fail colors
• 190c26c Change ConfirmPrompt struct fields to private. Prevent scrolling beyond the list. Fix color bleeding.
• 0b4f33c Use sort_unstable_by no need of sort_by here, fix word disappearing by recalculating the render x for preview text.
• 10352c6 Replace anstyle and text-wrap with ratatui inbuilt functions. Prevent processing of text every frame by caching the processed text.
  - 6e5fe07 Adds back the removed clap features from cargo toml. fix: CLI arguments not working #935
• 3d0f7f4 Reference instead of borrowing commands from state, Refactor draw function variables to immutable, calculate inner size from block instead of manual definition.
• 79aae9e Move tips to a seperate file for modularity. Pass the whole args to state instead of seperate args. Add the longest_tab_length to appstate struct so that it will not be calculated for each frame rendered. Use function for spawning confirmprompt. Merge command list and task items list rendering a single widget instead of two. Remove redundant keys in handle_key.
• 1b02fc3 Use const for theme functions.
• 1044517 Prevent getting locked out when in floating window.
• 78b7c3f Add more options to the config files

Testing

• Tested with no issues.

Before

After

Before

output.mp4

After

2024-11-10.21-38-15.mp4

Impact

Performance improvements and clean code base.

Issues / other PRs related

• Resolves Search text disappears if arrow keys are used #932
• Resolves main doesnt build #953
• Closes fix repo #954

Checklist

• My code adheres to the coding and style guidelines of the project.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• My changes generate no errors/warnings/merge conflicts.

[lj3954]

• 0b4f33c Use regex for case insensitive search, implement String instead of char<Vec> for seach_input, fix word disappearing by recalculating the render x for preview text.

Other than the last point, why would we want to do any of these things? Compiling a regex on each change to the search input makes the code less readable & negatively impacts performance compared to just taking the lowercase of the input, which we already do. Vec<char>, over String, is used to fix panics upon inputting a multi-byte unicode character, unicode_width is used to fix incorrect cursor positioning with wider unicode characters. Both are niche issues, but I don't see the purpose of removing these fixes for absolutely no improvement.

3b7e859 Replace redundant match statements with helper function in theme.rs.

Adding a new theme which has different colours for these theme elements could require these match statements be added back. This is the same thing you did when you suggested removing "unnecessary cases" in scripts. This change also does not improve readability.

Sorry, I misunderstood the changes made in that commit. It's even worse an idea than what I thought you were doing. If more themes are added, you'd have to keep adding function parameters, making the code even more difficult to read. A match statement is by far the best choice there.

I oppose these 2 commits. I'll review certain elements of the other changes in a few minutes.

[nnyyxxxx]

tab completion was purposefully made to complete lowercase for UX & better visuals on the users end

[nnyyxxxx]

i personally think its better to keep it lowercase as it feels more snappier that way imo, tldr: makes the user think they have more control when searching

[jeevithakannan2]

• 0b4f33c Use regex for case insensitive search, implement String instead of char<Vec> for seach_input, fix word disappearing by recalculating the render x for preview text.

Other than the last point, why would we want to do any of these things? Compiling a regex on each change to the search input makes the code less readable & negatively impacts performance compared to just taking the lowercase of the input, which we already do. Vec<char>, over String, is used to fix panics upon inputting a multi-byte unicode character, unicode_width is used to fix incorrect cursor positioning with wider unicode characters. Both are niche issues, but I don't see the purpose of removing these fixes for absolutely no improvement.

Thanks for the quick review. I forgot about the multi-byte characters reverted back to Vec<char> with some comments.

[jeevithakannan2]

@cartercanedy @lj3954 Why we are using ansi colors for tree-sitter-highlighting instead of ratatui colors?

[cartercanedy]

treesitter-highlight provides the machinery for interacting with the individual parsers for syntax highlighting, in our case it's the bash parser. This is necessary

Sorry, misunderstood the initial question

Anstlye makes it easier to render the ansi color codes directly to a stream. There's some complication with using the ratatui text constructs since there might be some styling that needs to be applied over a line break, which would make parsing more complicated than necessary

[jeevithakannan2]

Since we using custom colors for syntax_highlighting_styles maybe using the ratatui colors reduces the complexity?

What if ratatui colors is used and converted into lines using ratatui spans, lines or text instead of using anstyle and ansi_to_tui to convert them into text ?

[cartercanedy]

Trust me, it's just going to get more complicated...

[jeevithakannan2]

Ok I will try my best and create a commit let me know what you think about it

[jeevithakannan2]

Is it normal for the checkout process to take in 10min in typos workflow 🙄 https://github.com/ChrisTitusTech/linutil/actions/runs/11782804516/job/32818643344?pr=933

[cartercanedy]

The initial checkout can get throttled, I've seen that happen in my own projects

[jeevithakannan2]

The initial checkout can get throttled, I've seen that happen in my own projects

Are the changes to the anstyle good ? Did you review the changes?

[lj3954]

Making the theme functions const doesn't accomplish anything useful; they'll still be evaluated at runtime since the theme is not known at compile time.

[lj3954]

We also should not be merging PRs like this. I find it unlikely Chris will accept anything with a 1500 line diff.

[jeevithakannan2]

Making the theme functions const doesn't accomplish anything useful; they'll still be evaluated at runtime since the theme is not known at compile time.

It may not be now. But if we decide to change how themes work instead of passing the default theme at run time making it as default during compile time will make this more efficient.

[jeevithakannan2]

We also should not be merging PRs like this. I find it unlikely Chris will accept anything with a 1500 line diff.

I'm aware of that. What can we do? Do I create 9 separate PRs for each commit?

[cartercanedy]

Making the theme functions const doesn't accomplish anything useful; they'll still be evaluated at runtime since the theme is not known at compile time.

It may not be now. But if we decide to change how themes work instead of passing the default theme at run time making it as default during compile time will make this more efficient.

I don't mind the idea of marking methods const when compatible, but I don't see any way that the theme will ever become a compile-time constant as long as it remains configurable at runtime. The only exception would be if We could make this work if we constructed instances of each theme statically and just passed around references to the one of the compile-time generated themes

[cartercanedy]

We also should not be merging PRs like this. I find it unlikely Chris will accept anything with a 1500 line diff.

I'm aware of that. What can we do? Do I create 9 separate PRs for each commit?

Yes

[adamperkowski]

man that diff is HUGE
@jeevithakannan2, the conflict-creator

[jeevithakannan2]

man that diff is HUGE
@jeevithakannan2, the conflict-creator

I know we have put a lot of time into this. Separating the PRs will do the same. But this makes the program more efficient in many areas. Can't complain.

[adamperkowski]

Didn't expect you to actually merge this huge diff but ok.