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

For directories in preview column, dim cursor #924

Merged
merged 2 commits into from
Oct 16, 2022

Conversation

ilyagr
Copy link
Collaborator

@ilyagr ilyagr commented Aug 30, 2022

Reviewing note

This PR consists of two commits: one that does all the work with
a small diff, and another one that is a no-op and tries to make
the code easier to read.

PR Description

When preview is on and consists of a directory listing, make the
cursor in there dimmer to emphasize it's not active (as in the first screenshot below). This makes it
visually obvious which cursor is active at all times, without
having to read the path. The active cursor is now always the
rightmost bright one, regardless of whether the preview option
is on or off.

For example, you can easily and without reading anything tell the difference between the following two situations:

  1. Preview is on, we are in the second column (this PR in action).

    Preview on

  2. Preview is off, we are in the third column (same as before this PR).

    no_preview

If the preview is on but shows a file rather than a directory, this PR doesn't change anything.

In an ideal world, the color for the dim cursor would be customizable, but there is currently no framework for specifying these sorts of colors in lf (it could be a nonstandard extension to LS_COLORS, perhaps?). I picked a grey color with 50% brightness, which should be available on all terminals. I hope it will work OK for both bright and dark themes.

More motivation

Before this, I noticed that I often open a file by accident because
I got confused which of the three cursors on the screen is the active
one.

ilyagr added a commit to ilyagr/lf that referenced this pull request Aug 30, 2022
When preview is on and consists of a directory listing, make the
cursor in there dimmer to emphasize it's not active. This makes it
visually obvious which cursor is active at all times, without
having to read the path. The active cursor is now always  the
rightmost bright one, regardless of whether the `preview` option
is on or off.

See screenshots in the pull request at
gokcehan#924.

Before this, I noticed that I often open a file by accident because
I got confused which of the three cursors on the screen is the active
one.

In the future, I also hope this dimming could be used to show when an lf
window loses focus, based on @laktak's implementation of focus tracking
in gokcehan#861.
ilyagr added a commit to ilyagr/lf that referenced this pull request Sep 3, 2022
When preview is on and consists of a directory listing, make the
cursor in there dimmer to emphasize it's not active. This makes it
visually obvious which cursor is active at all times, without
having to read the path. The active cursor is now always  the
rightmost bright one, regardless of whether the `preview` option
is on or off.

See screenshots in the pull request at
gokcehan#924.

Before this, I noticed that I often open a file by accident because
I got confused which of the three cursors on the screen is the active
one.

In the future, I also hope this dimming could be used to show when an lf
window loses focus, based on @laktak's implementation of focus tracking
in gokcehan#861.
@ilyagr
Copy link
Collaborator Author

ilyagr commented Sep 3, 2022

Just pushed a minor update (improved naming).

@ilyagr
Copy link
Collaborator Author

ilyagr commented Sep 11, 2022

Added a bugfix so that empty directories and similar situations get the same highlighting as non-empty ones.

ilyagr added a commit to ilyagr/lf that referenced this pull request Sep 20, 2022
This mainly merges gokcehan#924 with @laktak's
gokcehan#861. This also merges in the `:keys`
command (gokcehan#918) as well as 
and some unrelated minor corrections (currently
dd5dc59).
ilyagr added a commit to ilyagr/lf that referenced this pull request Oct 9, 2022
This mainly merges gokcehan#924 with @laktak's
gokcehan#861. This also merges in the `:keys`
command (gokcehan#918) as well as 
and some unrelated minor corrections (currently
dd5dc59).
@ilyagr
Copy link
Collaborator Author

ilyagr commented Oct 9, 2022

Synced to master, reduced back to two commits (one for functionality, one for a no-op cleanup).

@gokcehan
Copy link
Owner

@ilyagr I'm going over patches as part of #950 as you are aware. Can you update this patch to resolve conflicts?

When preview is on and consists of a directory listing, make the
cursor in there dimmer to emphasize it's not active. This makes it
visually obvious which cursor is active at all times, without
having to read the path. The active cursor is now always  the
rightmost bright one, regardless of whether the `preview` option
is on or off.

See screenshots in the pull request at
gokcehan#924.

Before this, I noticed that I often open a file by accident because
I got confused which of the three cursors on the screen is the active
one.
Reduce number of arguments to printDir and list non-configurable colors
@ilyagr
Copy link
Collaborator Author

ilyagr commented Oct 15, 2022

@gokcehan Done, thanks for looking!

@ilyagr
Copy link
Collaborator Author

ilyagr commented Oct 16, 2022

I was also considering adding an option to choose whether the preview dir cursor is dim (as implemented here), normal (as it was before), or invisible. I didn't do it since I think it's better to have fewer options and a good default behavior whenever possible, but it remains a possibility.

@gokcehan
Copy link
Owner

@ilyagr Thanks for the patch.

@gokcehan gokcehan merged commit 04d4083 into gokcehan:master Oct 16, 2022
@ilyagr ilyagr deleted the dim-preview-cursor branch October 18, 2022 05:31
gokcehan added a commit that referenced this pull request Dec 24, 2022
ilyagr added a commit to ilyagr/lf that referenced this pull request Jan 19, 2023
…w panes

This allows using a different style for the normal and preview cursor. See
linked issues and PRs for the context.

The default behavior is an underline, same as in
gokcehan#1072, since it should not cause severe
problems and be visible on all terminals. It can now be easily changed.

Fixes gokcehan#1038

Follows up on:

gokcehan#1072
gokcehan#924
gokcehan@b47cf6d5a5
ilyagr added a commit to ilyagr/lf that referenced this pull request Jan 19, 2023
This defines new `cursorfmt` and `crusorpreviewfmt` options to style cursor in normal/preview panes. This allows using a different style for the normal and preview cursor. The documentation includes several possible values these options can be set to.

The default behavior is an underline, same as in
gokcehan#1072, since it should not cause severe
problems and be visible on all terminals. It can now be easily changed, as explained in the docs. I also added an example to `etc/lfrc.example`.

See linked issues and PRs for the more context.

Fixes gokcehan#1038

Follows up on:

gokcehan#1072
gokcehan#924
gokcehan@b47cf6d5a5
ilyagr added a commit to ilyagr/lf that referenced this pull request Jan 19, 2023
This defines new `cursorfmt` and `crusorpreviewfmt` options to style cursor in normal/preview panes. This allows using a different style for the normal and preview cursor. The documentation includes several possible values these options can be set to.

The default behavior is an underline, same as in
gokcehan#1072, since it should not cause severe
problems and be visible on all terminals. It can now be easily changed, as explained in the docs. I also added an example to `etc/lfrc.example`.

See linked issues and PRs for the more context.

Fixes gokcehan#1038

Follows up on:

gokcehan#1072
gokcehan#924
gokcehan@b47cf6d5a5
ilyagr added a commit to ilyagr/lf that referenced this pull request Jan 19, 2023
This defines new `cursorfmt` and `crusorpreviewfmt` options to style cursor in normal/preview panes. This allows using a different style for the normal and preview cursor. The documentation includes several possible values these options can be set to.

The default behavior is an underline, same as in
gokcehan#1072, since it should not cause severe
problems and be visible on all terminals. It can now be easily changed, as explained in the docs. I also added an example to `etc/lfrc.example`.

See linked issues and PRs for the more context.

Fixes gokcehan#1038

Follows up on:

gokcehan#1072
gokcehan#924
gokcehan@b47cf6d5a5
ilyagr added a commit to ilyagr/lf that referenced this pull request Jan 20, 2023
This defines new `cursorfmt` and `crusorpreviewfmt` options to style cursor in normal/preview panes. This allows using a different style for the normal and preview cursor. The documentation includes several possible values these options can be set to.

The default behavior is an underline, same as in
gokcehan#1072, since it should not cause severe
problems and be visible on all terminals. It can now be easily changed, as explained in the docs. I also added an example to `etc/lfrc.example`.

See linked issues and PRs for the more context.

Fixes gokcehan#1038

Follows up on:

gokcehan#1072
gokcehan#924
gokcehan@b47cf6d5a5
ilyagr added a commit to ilyagr/lf that referenced this pull request Jan 28, 2023
This restores functionality from gokcehan#924
reverted in gokcehan@b47cf6d5a5, while
fixing gokcehan#1038.

Unlike before, the cursor in the preview column is now an underline
rather than a grey background. This seems to work on most terminals.
In no case should the visibility of the file name itself be affected.

For messages such as "empty" or "permission denied", the preview
cursor is the same as the active cursor for now.

There is no configuration, but there is nothing preventing adding
config as discussed in gokcehan#1038
in the future.

![Screenshot](https://user-images.githubusercontent.com/4123047/209767426-abcbddd4-965a-43a8-a5a8-6242681bbd98.png)
gokcehan pushed a commit that referenced this pull request Feb 11, 2023
…y `tagfmt` and `errorfmt` (#1086)

* Allow separate styles for cursor in normal and preview panes

This defines new `cursorfmt` and `crusorpreviewfmt` options to style cursor in normal/preview panes. This allows using a different style for the normal and preview cursor. The documentation includes several possible values these options can be set to.

The default behavior is an underline, same as in
#1072, since it should not cause severe
problems and be visible on all terminals. It can now be easily changed, as explained in the docs. I also added an example to `etc/lfrc.example`.

See linked issues and PRs for the more context.

Fixes #1038

Follows up on:

#1072
#924
b47cf6d5a5

* Simplify configuration of several formatting options

This simplifies the configuration of `cursorfmt`, `cursorpreviewfmt`, `tagfmt`,
and `errorfmt` options.

In almost all usecases, the value of these options had to end with `%s\033[0m`
to print the contents of filename/tag/error and reset the terminal style. Now,
if `%s` is not part of the option's value, `%s\033[0m` is appended
automatically. This simplifies configuration.

I retained the same behavior as before when `%s` is part of the string for
backwards compatibility. I think it should be considered deprecated, but it
also causes no harm.
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