On Wed, Jul 22, 2020 at 02:58:50PM +0300, Maxim Tarasov wrote:
> -static void print_enriched_string (int attr, unsigned char *s, int do_color) > +static void print_enriched_string (int base_color, unsigned char *s, int do_color, int is_current)I don't like the parameter to be named is_current but doesn't reflect that fact in the callers.I agree. At first I just wanted to reduce the amount of option handling and branching in print_enriched_string, but wasn't able to get rid of it completely.Now that you mention it, I've realized that do_color parameter isn't used properly as well. print_enriched_string can always do its own coloring without relying on indicator being applied already. The only bit of information it needs is whether to use indicator on top of base colors.
Hi Maxim,I've only taken a quick look at the patch - I don't have time to look closely or test right now.
It looks like this version is now changing behavior with $cursor_overlay unset. The tree colors should not be drawn (with $arrow_cursor unset)
in that case. Would you confirm that is still the case.
To further simplify option checking I've moved OPTCURSOROVERLAY check into mutt_merge_colors.
Personally I'd rather leave it as before, with mutt_merge_colors() doing what it is told, in case that needs to be used in other places. Whereas mutt_attrset_cursor() sets the cursor how is should based on options.
However if the $cursor_overlay is now going to affect tree drawing too, it may be more appropriate the name the option $color_overlay. What do you think?I didn't think have the checks in print_enriched_string(), where appropriate, was too bad, but you can write a helper for that.
-Kevin -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
