bakkeby / st-flexipatch

An st build with preprocessor directives to decide which patches to include during build time
MIT License
348 stars 107 forks source link

Sixel preview broken in `48c85cdcf5ec` #145

Closed myrrc closed 1 month ago

myrrc commented 1 month ago

Hello and thanks for maintaining this repository. My image preview with chafa -f stopped working. Result of bisect is the following commit on which the image preview breaks -- image size is drawn correctly, but instead of the image blank cells are drawn. Before this commit everything worked fine.

myrrc commented 1 month ago
ъ chafa --version
Chafa version 1.14.0
Loaders:  AVIF GIF JPEG PNG QOI SVG TIFF WebP XWD
Features: mmx sse4.1 popcnt avx2
Applying: mmx sse4.1 popcnt avx2

Debian trixie

veltza commented 1 month ago

Could you please paste your patches.h file so we can see what patches you are using? Does this happen when you run chafa from the command line or do you use some terminal file manager?

myrrc commented 1 month ago

This happens both in command-line invocations of chafa as well as chafa + ctpv previews in lf file manager.

~ git diff --no-color origin/master -- patches.def.h | grep define

define BAR_TAGPREVIEW_PATCH 0

-#define BAR_STATUS_PATCH 1 +#define BAR_STATUS_PATCH 0

define BAR_SYSTRAY_PATCH 0

-#define BAR_TAGS_PATCH 1 +#define BAR_TAGS_PATCH 0

define BAR_WINICON_PATCH 0

-#define BAR_WINTITLE_PATCH 1 +#define BAR_WINTITLE_PATCH 0 -#define CENTER_TRANSIENT_WINDOWS_PATCH 0 +#define CENTER_TRANSIENT_WINDOWS_PATCH 1 -#define CENTER_TRANSIENT_WINDOWS_BY_PARENT_PATCH 0 +#define CENTER_TRANSIENT_WINDOWS_BY_PARENT_PATCH 1 -#define COOL_AUTOSTART_PATCH 0 +#define COOL_AUTOSTART_PATCH 1 -#define DECORATION_HINTS_PATCH 0 +#define DECORATION_HINTS_PATCH 1 -#define LOSEFULLSCREEN_PATCH 0 +#define LOSEFULLSCREEN_PATCH 1 -#define NOBORDER_PATCH 0 +#define NOBORDER_PATCH 1 -#define STEAM_PATCH 0 +#define STEAM_PATCH 1 -#define TOGGLEFULLSCREEN_PATCH 0 +#define TOGGLEFULLSCREEN_PATCH 1 -#define VANITYGAPS_PATCH 0 +#define VANITYGAPS_PATCH 1

veltza commented 1 month ago

I need the patches.h from your st-flexipatch, not dwm-flexipatch.

myrrc commented 1 month ago

Sorry for that :)

~ git diff --no-color origin/master -- patches.def.h | grep +#define +#define BOXDRAW_PATCH 1 +#define CLIPBOARD_PATCH 1 +#define COLUMNS_PATCH 1 +#define COPYURL_PATCH 1 +#define COPYURL_HIGHLIGHT_SELECTED_URLS_PATCH 1 +#define CSI_22_23_PATCH 1 +#define DYNAMIC_CURSOR_COLOR_PATCH 1 +#define EXTERNALPIPE_PATCH 1 +#define EXTERNALPIPEIN_PATCH 1 +#define HIDECURSOR_PATCH 1 +#define KEYBOARDSELECT_PATCH 1 +#define LIGATURES_PATCH 1 +#define REFLOW_PATCH 1 +#define SCROLLBACK_PATCH 1 +#define SCROLLBACK_MOUSE_ALTSCREEN_PATCH 1 +#define SIXEL_PATCH 1 +#define WIDE_GLYPHS_PATCH 1

veltza commented 1 month ago

You should not use both the column and the reflow patches at the same time, because the reflow makes the column patch useless. Using both can only cause issues. So try disabling the column patch and see if that fixes the issue.

You can also disable the scrollback patch when the reflow is enabled.

myrrc commented 1 month ago

Thanks for the advice, I disabled these patches, and everything works fine.

veltza commented 1 month ago

Great!

When I checked the code, I realized that I had anticipated that someone might use both of those patches and tried to manage the situation gracefully, but I had written a bug in the code. Doh.

I need to submit a fix for that silly bug.

myrrc commented 1 month ago

I can create a PR with changes to patches.def.h stating one shouldn't use some patches together e.g. column and reflow, or reflow and scrollback"

bakkeby commented 1 month ago

I don't think that will be particularly necessary. For the most part the reflow patch should take precedence over the older columns patch.

The columns patch is now more of a curiosity, quite an improvement over the default behaviour of just cutting text when the window is resized, but is less practical than the reflow patch.

It would probably make more sense to remove the columns patch going forward.