Skip to content

Ticket #4685: fix filename background color when scrolling #4685

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Ticket #4685: fix filename background color when scrolling #4685

wants to merge 7 commits into from
+47 −35

Conversation

zyv
Copy link
Member

@zyv zyv commented Apr 13, 2025 •
edited
Loading

Proposed changes

The filename scrolling feature has a background coloring bug:

Full listing Long listing User listing
< before-full before-long before-user
> after-full after-long after-user

Refs: #2731

@zyv zyv requested a review from aborodin April 13, 2025 09:13
@zyv zyv self-assigned this Apr 13, 2025
@github-actions github-actions bot added needs triage Needs triage by maintainers prio: medium Has the potential to affect progress labels Apr 13, 2025
@github-actions github-actions bot added this to the Future Releases milestone Apr 13, 2025
@zyv zyv modified the milestones: Future Releases, 4.8.34 Apr 13, 2025
@zyv zyv added area: core Issues not related to a specific subsystem and removed needs triage Needs triage by maintainers labels Apr 13, 2025
@zyv zyv changed the title Ticket #XXXX: fix filename background color when scrolling Ticket #4685: fix filename background color when scrolling Apr 13, 2025
@zyv zyv force-pushed the feature/fix-filename-scrolling-color branch from 23cab17 to 63443f4 Compare April 13, 2025 09:15
@zyv zyv mentioned this pull request Apr 13, 2025
Merged
zyv and others added 2 commits April 13, 2025 14:59
@zyv
keymaps: add default ScrollLeft / ScrollRight bindings
26be882 
Signed-off-by: Yury V. Zaytsev <[email protected]>
@aborodin
Fix display of panel_filename_scroll_left_char.
fd01deb 
  * (format_file): set FILENAME_SCROLL_LEFT flag if filename scroll
    offset is more than zero.
  * (repaint_file): display panel_filename_scroll_left_char if
    FILENAME_SCROLL_LEFT flag is set.

Signed-off-by: Andrew Borodin <[email protected]>
@mc-worker
Copy link

Added commit with small fix.

@mc-worker
Copy link

/approve

@mc-worker mc-worker added the state: approved If not using PRs: associated branch has been approved label Apr 13, 2025
@zyv
Copy link
Member Author

zyv commented Apr 13, 2025

Added commit with small fix.

So I actually thought about adding the exact same "fix" you did, but then I realized that this was probably done on purpose. Apparently, this feature was implemented to be completely hidden:

Initially, panel->content_shift == -1 and not 0. So you "enable" it by asking it to shift to the right once, even though it doesn't actually shift anything. That is why both indicators ({ & }) appear. You "disable" it by shifting left to -1, so both indicators disappear.

I think that in FAR Manager this behavior is always unconditionally enabled. What do you think about enabling this feature by setting panel->content_shift == 0 by default and leaving the traditional collapsing in the status bar?

I think right now the discoverability of this feature is zero. I didn't even know it existed. But I actually like it better than collapsing, and if collapsing stays in all other places, hopefully this would not make people unhappy.

@zyv
Copy link
Member Author

zyv commented Apr 13, 2025

/approve

Ha-ha, there is no end to this. Do you know why this failed now? Because @mc-worker is a private member of the organization, unlike @aborodin (or me). So GitHub cannot really check if you are authorized to post the comment o_O If you set it to public in @mc-worker profile, it should finally work :)

aborodin added 4 commits April 20, 2025 11:23
@aborodin
fixup! Fix display of panel_filename_scroll_left_char.
f563dd5
@aborodin
(format_file): minor optimization.
822a35a 
Signed-off-by: Andrew Borodin <[email protected]>
@aborodin
(format_file): rename variables.
de5ffe5 
Signed-off-by: Andrew Borodin <[email protected]>
@aborodin
Panel: enable content shift mode by default.
1853e3e 
  * (WPanel): make content_shift and max_shift members unsigned.
  * (panel_sized_empty_new): init content_shift and max_shift members by
    zero.
  * (pamel_clean_dir): likewise.
  * (paint_dir): init max_shift by zero.
  * (format_file): simplify tests. Since content_shift and max_shift are
    unsigned, some conditions are always true.
  * (panel_content_scroll_left): likewise.
  * (panel_content_scroll_right): likewise.
@mc-worker
Copy link

I think that in FAR Manager this behavior is always unconditionally enabled. What do you think about enabling this feature by setting panel->content_shift == 0 by default and leaving the traditional collapsing in the status bar?

Done. Please test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aborodin aborodin Awaiting requested review from aborodin

Assignees

@zyv zyv

Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress state: approved If not using PRs: associated branch has been approved
Milestone
4.8.34
Development

Successfully merging this pull request may close these issues.
3 participants
@zyv @mc-worker @aborodin