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

refactor: avoid strange round-trip of draw_fn arguments #1922

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Integral-Tech
Copy link

  • Although format! macro is very powerful, it introduces overhead of memory allocation on the heap compared to &str. Therefore, when format! macro is unnecessary, we should avoid using it.

@th1000s
Copy link
Collaborator

th1000s commented Dec 9, 2024

I noticed that this pattern repeats itself:

let (draw_fn, pad, style) = draw::get_draw_function();

draw_fn(pad, style, ...);

The get returns a function/closure and parameters for it, so those are (directly or indirectly) fed back into it. Instead the returned closure should already handle these, then the if pad { .. } else { .. } is only inside the closure, and decoration_ansi_term_style doesn't make this strange round-trip.

Also, type DrawFunction doesn't have to be a dyn FnMut, just dyn Fn. However, the padding in merge_conflict.rs would also get applied to derived_commit_name, but maybe this visually doesn't make a difference.

@Integral-Tech Integral-Tech changed the title refactor: avoid using format! when it is unnecessary refactor: avoid strange round-trip of draw_fn arguments Dec 17, 2024
@Integral-Tech
Copy link
Author

@th1000s I have just submitted a commit to fix the strange round-trip :)

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