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 filename truncation code #1272

Merged
merged 2 commits into from
Jun 4, 2023
Merged

Refactor filename truncation code #1272

merged 2 commits into from
Jun 4, 2023

Conversation

joelim-work
Copy link
Collaborator

This change is a follow-up of #1150, and isolates the code for handling the filename truncation so that it is done in only one place. Previously the filename would be truncated if it would overflow the pane, but if there was file info to be displayed, the filename would have to be truncated again.

@joelim-work
Copy link
Collaborator Author

Incidentally this also fixes a bug where the file info is misaligned when the last rune is supposed to be a double-width character and is replaced by the truncate character (which is single-width).

Before:
image

After:
image

Copy link
Collaborator

@ilyagr ilyagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice! I am not 100% this is bug free, but I'm pretty comfortable with it. I think any bugs here would be pretty obvious for someone running the code.

I might try it out some more and/or look at the Rune functions this calls.

All of my comments are about more things we could refactor, but it might make sense to postpone that to another PR.

ui.go Show resolved Hide resolved
ui.go Show resolved Hide resolved
@gokcehan
Copy link
Owner

gokcehan commented Jun 4, 2023

Thanks @joelim-work for the patch and thanks @ilyagr for the review.

@gokcehan gokcehan merged commit 7c33da2 into gokcehan:master Jun 4, 2023
@joelim-work joelim-work deleted the refactor-filename-truncate branch June 4, 2023 23:14
@gokcehan gokcehan mentioned this pull request Sep 17, 2023
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.

3 participants