Repository: gokcehan/lf
Pull request #1029 (2022-11-27 11:27:42)

Flexible filename truncation
Inspired how [MidnightCommander](https://github.com/MidnightCommander/mc/) display too long filenames, I would like to propose a new config option *fnametruncpct* to allow flexible truncation of too long filenames, i.e. not only at the end like before, but also in the middle of a loo long filename, e.g.:

- `set fnametruncpct 100` ->  "very-long-filename-tr~"
- `set fnametruncpct 50` -> "very-long-f~-truncated"
- `set fnametruncpct 0`   -> "~ng-filename-truncated"

The additional fix commit was necessary to fix a presumed bug in `runeSliceWidthRange()` discovered during developing this change.

Let me know what you think.
[I'm especially not sure if the option name *fnametruncpct* fits well (I would have made it more verbose, i.e. longer, like *filename_truncate_percent*, all others are much shorter and without underlines).]

laktak: This is an interesting idea but I think you could improve upon it.

For example if you have long filenames that match in some places then the algorithm should work this out and show the parts that differ.

Having a fixed percentage can't do this and it wouldn't be able to show the relevant parts. IMO this implementation (with a fixed percentage) does not offer enough benefits over the current version.

gokcehan: @raslop Thanks for the patch but there is already a lot of complexity with truncation due to having multiple options for configuration (e.g. `info`, `number`). Here is an example when I tried the patch for a while:

![image](https://user-images.githubusercontent.com/1835672/204334193-926a84ea-a012-4405-af71-9e51f2f7adcd.png)

Even if you make it work I also don't see much benefit in this feature that will make the additional complexity acceptable. For many of the usecases I can image there are other ways to handle (i.e. switching to single column when filenames are long).
gokcehan: @raslop Here's an example with the latest code:

![image](https://user-images.githubusercontent.com/1835672/204382481-4b82cf24-95d9-4981-b25b-afe7aa6d5c3f.png)

This should be enough to argue that the complexity isn't worth it.
gokcehan: > I guess you are talking about the misplaced and duplicated line-number 1, right? I was not able to reproduce that, maybe it would help if you also tell the options used for lf in the screenshot. If you are talking about something else, please be more specific.

@raslop The issue is that `info` includes `size` but the last column of sizes are not shown. Misplaced number is related to `relativenumber` option. I did not mention it because it was not my intention to ask you to fix the problem. 

> I don't think so, at least not completely. It just tells that I haven't understood this part of the code sufficiently well.

Isn't that the problem? Now that you have been working on this part of the code for the last couple of days, you probably have a much better understanding of the code than anyone else currently. Personally, I don't remember anything about this part of the code. You will be in the same spot as me in 3 months or so. If something pops up, there won't be anyone to fix it without bashing their head. This is what I mean by complexity.

> As I want this feature, I'm willing to invest more time to finish it, until you it's good enough for you. I think it's not too far anymore. If you are willing to give further feedback, I would like to continue.

My only aim to have a rock solid stable software. All I did here was to play around for a few minutes to come up with these bugs. Previously, some of the bugs related to this part of the code were not even be discovered for a long time. The way you test these things is to systematically change options and parameters. For example if there are 3 boolean options, you may need to evaluate 8 different use cases by also changing the terminal size dynamically at the same time. This means an exponentially growing number of use cases. Currently, I think we already have many more options (e.g. `number`, `relativenumber`, different combinations of `info`, `dircounts`, maybe `infotimefmt` etc). You want to introduce another option which is not even a boolean so I'm not sure how it can even be tested properly. You also change a function that is used in other places (i.e. `runeSliceWidthRange`). As I'm not interested in this feature myself, unfortunately I will not be available for testing these things.

If you like, you can keep the PR open for a while for interested users to test it. It can also be useful to see if there is enough demand for this feature by others. With enough people, I think I can usually be pressured into accepting a patch.

Nowadays, I'm hoping to push a new release if I can motivate myself into composing a changelog. I'm not planning to include any other features before the release.
gokcehan: @raslop Your attention to detail seems more than I'm looking for. Ping me when you are ready and I will take a look at this again.

And sorry for being uptight about these things. I have spent a lot of time fixing broken patches of others in the past and I don't want to do it anymore.
joelim-work: Hi @raslop, if you really want to have this feature accepted, I think it's best to split the task into two parts, one to refactor the code and another to actually implement the feature. I have already submitted a PR for the refactor part, see #1272. This isolates the filename truncation code into one location so that it is easier to work with, but maintains the same intended behaviour without adding any features. Hopefully once that gets merged, you can then implement your feature on top of it.

Apologies for intruding upon your PR, but this cleanup was something I wanted to do regardless. Looking at the comment history in this PR, I think it was quite unfortunate you chose to work on this before I cleaned up the `printDir` code in #1150. The previous implementation before that had a number of bugs and most likely caused you a lot of pain, which is a shame because I think your feature is actually quite simple, but the conditions at the time made things quite hard for you to implement.
joelim-work: @raslop Thanks once again for your patience, I can see the changes `ui.go` look much nicer now. I have left a few comments that you might like to address, but I will leave it up to @gokcehan to decide whether this feature is worth accepting.
gokcehan: > [I'm still not sure if the option name fnametruncpct fits well (I would have made it more verbose, i.e. longer, like filename_truncate_percent, all others are much shorter and without underlines).]

@raslop We already have a `truncatechar` option so I think maybe you can drop the `fname` prefix and expand `trunc` as `truncate` to be consistent (i.e. `truncatepct`).

Note, I will not be around for a couple of weeks, but I will try to take a look at this again when I get access to a computer again.

Patch:
diff --git a/complete.go b/complete.go
index cae5082..3d91186 100644
--- a/complete.go
+++ b/complete.go
@@ -218,6 +218,7 @@ var (
 		"infotimefmtnew",
 		"infotimefmtold",
 		"truncatechar",
+		"truncatepct",
 	}
 )
 
diff --git a/doc.go b/doc.go
index 806b6aa..782f167 100644
--- a/doc.go
+++ b/doc.go
@@ -172,6 +172,7 @@ The following options can be used to customize the behavior of lf:
 	tempmarks        string    (default '')
 	timefmt          string    (default 'Mon Jan _2 15:04:05 2006')
 	truncatechar     string    (default '~')
+	truncatepct      int       (default 100)
 	waitmsg          string    (default 'Press any key to continue')
 	wrapscan         bool      (default true)
 	wrapscroll       bool      (default false)
@@ -941,6 +942,23 @@ Format string of the file modification time shown in the bottom line.
 
 Truncate character shown at the end when the file name does not fit to the pane.
 
+	truncatepct  int       (default 100)
+
+When a filename is too long to be shown completely, the available space is
+partitioned in two pieces. truncatepct defines a fraction (in percent
+between 0 and 100) for the size of the first piece, which will show the
+beginning of the filename. The second piece will show the end of the filename
+and will use the rest of the available space. Both pieces are separated by the
+truncation character (truncatechar).
+A value of 100 will only show the beginning of the filename,
+while a value of 0 will only show the end of the filename, e.g.:
+
+- `set truncatepct 100` -> "very-long-filename-tr~" (default)
+
+- `set truncatepct 50`  -> "very-long-f~-truncated"
+
+- `set truncatepct 0`   -> "~ng-filename-truncated"
+
 	waitmsg        string    (default 'Press any key to continue')
 
 String shown after commands of shell-wait type.
diff --git a/docstring.go b/docstring.go
index 61692de..24f6275 100644
--- a/docstring.go
+++ b/docstring.go
@@ -175,6 +175,7 @@ The following options can be used to customize the behavior of lf:
     tempmarks        string    (default '')
     timefmt          string    (default 'Mon Jan _2 15:04:05 2006')
     truncatechar     string    (default '~')
+    truncatepct      int       (default 100)
     waitmsg          string    (default 'Press any key to continue')
     wrapscan         bool      (default true)
     wrapscroll       bool      (default false)
@@ -1013,6 +1014,22 @@ Format string of the file modification time shown in the bottom line.
 
 Truncate character shown at the end when the file name does not fit to the pane.
 
+    truncatepct  int       (default 100)
+
+When a filename is too long to be shown completely, the available space is
+partitioned in two pieces. truncatepct defines a fraction (in percent between
+0 and 100) for the size of the first piece, which will show the beginning of
+the filename. The second piece will show the end of the filename and will use
+the rest of the available space. Both pieces are separated by the truncation
+character (truncatechar). A value of 100 will only show the beginning of the
+filename, while a value of 0 will only show the end of the filename, e.g.:
+
+- 'set truncatepct 100' -> "very-long-filename-tr~" (default)
+
+- 'set truncatepct 50' -> "very-long-f~-truncated"
+
+- 'set truncatepct 0' -> "~ng-filename-truncated"
+
     waitmsg        string    (default 'Press any key to continue')
 
 String shown after commands of shell-wait type.
diff --git a/eval.go b/eval.go
index 24975c3..2eea96d 100644
--- a/eval.go
+++ b/eval.go
@@ -831,6 +831,17 @@ func (e *setExpr) eval(app *app, args []string) {
 		}
 
 		gOpts.truncatechar = e.val
+	case "truncatepct":
+		n, err := strconv.Atoi(e.val)
+		if err != nil {
+			app.ui.echoerrf("truncatepct: %s", err)
+			return
+		}
+		if n < 0 || n > 100 {
+			app.ui.echoerrf("truncatepct: must be between 0 and 100 (both inclusive), got %d", n)
+			return
+		}
+		gOpts.truncatepct = n
 	case "waitmsg":
 		gOpts.waitmsg = e.val
 	case "wrapscan":
diff --git a/lf.1 b/lf.1
index 561edc5..2804db4 100644
--- a/lf.1
+++ b/lf.1
@@ -191,6 +191,7 @@ The following options can be used to customize the behavior of lf:
     tempmarks        string    (default '')
     timefmt          string    (default 'Mon Jan _2 15:04:05 2006')
     truncatechar     string    (default '~')
+    truncatepct      int       (default 100)
     waitmsg          string    (default 'Press any key to continue')
     wrapscan         bool      (default true)
     wrapscroll       bool      (default false)
@@ -1119,6 +1120,18 @@ Format string of the file modification time shown in the bottom line.
 .PP
 Truncate character shown at the end when the file name does not fit to the pane.
 .PP
+.EX
+    truncatepct  int       (default 100)
+.EE
+.PP
+When a filename is too long to be shown completely, the available space is partitioned in two pieces. truncatepct defines a fraction (in percent between 0 and 100) for the size of the first piece, which will show the beginning of the filename. The second piece will show the end of the filename and will use the rest of the available space. Both pieces are separated by the truncation character (truncatechar). A value of 100 will only show the beginning of the filename, while a value of 0 will only show the end of the filename, e.g.:
+.PP
+- `set truncatepct 100` -> "very-long-filename-tr~" (default)
+.PP
+- `set truncatepct 50` -> "very-long-f~-truncated"
+.PP
+- `set truncatepct 0` -> "~ng-filename-truncated"
+.PP
 .EX
     waitmsg        string    (default 'Press any key to continue')
 .EE
diff --git a/misc.go b/misc.go
index b0303ea..8199697 100644
--- a/misc.go
+++ b/misc.go
@@ -53,6 +53,19 @@ func runeSliceWidthRange(rs []rune, beg, end int) []rune {
 	return rs[b:]
 }
 
+// Returns the last runes of `rs` that take up at most `maxWidth` space.
+func runeSliceWidthLastRange(rs []rune, maxWidth int) []rune {
+	lastWidth := 0
+	for i := len(rs) - 1; i >= 0; i-- {
+		w := runewidth.RuneWidth(rs[i])
+		if lastWidth+w > maxWidth {
+			return rs[i+1:]
+		}
+		lastWidth += w
+	}
+	return rs
+}
+
 // This function is used to escape whitespaces and special characters with
 // backlashes in a given string.
 func escape(s string) string {
diff --git a/opts.go b/opts.go
index 21d624c..5b8feaa 100644
--- a/opts.go
+++ b/opts.go
@@ -72,6 +72,7 @@ var gOpts struct {
 	infotimefmtnew   string
 	infotimefmtold   string
 	truncatechar     string
+	truncatepct      int
 	ratios           []int
 	hiddenfiles      []string
 	history          bool
@@ -133,6 +134,7 @@ func init() {
 	gOpts.infotimefmtnew = "Jan _2 15:04"
 	gOpts.infotimefmtold = "Jan _2  2006"
 	gOpts.truncatechar = "~"
+	gOpts.truncatepct = 100
 	gOpts.ratios = []int{1, 2, 3}
 	gOpts.hiddenfiles = []string{".*"}
 	gOpts.history = true
diff --git a/os.go b/os.go
index d26b9f1..46ee694 100644
--- a/os.go
+++ b/os.go
@@ -10,7 +10,6 @@ import (
 	"os/user"
 	"path/filepath"
 	"runtime"
-	"strconv"
 	"strings"
 	"syscall"
 
@@ -210,7 +209,7 @@ func groupName(f os.FileInfo) string {
 
 func linkCount(f os.FileInfo) string {
 	if stat, ok := f.Sys().(*syscall.Stat_t); ok {
-		return strconv.FormatUint(stat.Nlink, 10)
+		return fmt.Sprint(stat.Nlink)
 	}
 	return ""
 }
diff --git a/ui.go b/ui.go
index 635a428..4b8cb55 100644
--- a/ui.go
+++ b/ui.go
@@ -468,8 +468,11 @@ func (win *win) printDir(screen tcell.Screen, dir *dir, context *dirContext, dir
 
 		filename := []rune(f.Name())
 		if runeSliceWidth(filename) > maxFilenameWidth {
-			filename = runeSliceWidthRange(filename, 0, maxFilenameWidth-1)
+			truncatePos := (maxFilenameWidth - 1) * gOpts.truncatepct / 100
+			lastPart := runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)
+			filename = runeSliceWidthRange(filename, 0, truncatePos)
 			filename = append(filename, []rune(gOpts.truncatechar)...)
+			filename = append(filename, lastPart...)
 		}
 		for i := runeSliceWidth(filename); i < maxFilenameWidth; i++ {
 			filename = append(filename, ' ')


Test patch:
diff --git a/misc_test.go b/misc_test.go
index aa6f6df..758e342 100644
--- a/misc_test.go
+++ b/misc_test.go
@@ -83,6 +83,47 @@ func TestRuneSliceWidthRange(t *testing.T) {
 	}
 }
 
+func TestRuneSliceWidthLastRange(t *testing.T) {
+	tests := []struct {
+		rs       []rune
+		maxWidth int
+		exp      []rune
+	}{
+		{[]rune{}, 0, []rune{}},
+		{[]rune{}, 1, []rune{}},
+		{[]rune{'a', 'ı', 'b', ' '}, 0, []rune{}},
+		{[]rune{'a', 'ı', 'b', ' '}, 1, []rune{' '}},
+		{[]rune{'a', 'ı', 'b', ' '}, 2, []rune{'b', ' '}},
+		{[]rune{'a', 'ı', 'b', ' '}, 3, []rune{'ı', 'b', ' '}},
+		{[]rune{'a', 'ı', 'b', ' '}, 4, []rune{'a', 'ı', 'b', ' '}},
+		{[]rune{'a', 'ı', 'b', ' '}, 5, []rune{'a', 'ı', 'b', ' '}},
+		{[]rune{'世', '界', '世', '界'}, 0, []rune{}},
+		{[]rune{'世', '界', '世', '界'}, 1, []rune{}},
+		{[]rune{'世', '界', '世', '界'}, 2, []rune{'界'}},
+		{[]rune{'世', '界', '世', '界'}, 3, []rune{'界'}},
+		{[]rune{'世', '界', '世', '界'}, 4, []rune{'世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 5, []rune{'世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 6, []rune{'界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 7, []rune{'界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 8, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 9, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 0, []rune{}},
+		{[]rune{'世', 'a', '界', 'ı'}, 1, []rune{'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 2, []rune{'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 3, []rune{'界', 'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 4, []rune{'a', '界', 'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 5, []rune{'a', '界', 'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 6, []rune{'世', 'a', '界', 'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 7, []rune{'世', 'a', '界', 'ı'}},
+	}
+
+	for _, test := range tests {
+		if got := runeSliceWidthLastRange(test.rs, test.maxWidth); !reflect.DeepEqual(got, test.exp) {
+			t.Errorf("at input '%v' expected '%v' but got '%v'", test.rs, test.exp, got)
+		}
+	}
+}
+
 func TestEscape(t *testing.T) {
 	tests := []struct {
 		s   string