Flexible filename truncation #1029

raslop · 2022-11-27T11:27:42Z

Inspired how MidnightCommander 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 · 2022-11-27T20:52:58Z

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.

raslop · 2022-11-27T21:48:38Z

This is an interesting idea but I think you could improve upon it.

Thanks 😃

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.

Sure, there's always room for improvement. However, I see the following points with this suggestion, which is basically an extension of the requirements:

Usability: The truncation marker itself is sometimes hard to notice in the middle of the filename. However, if this feature is used, it has been consciously configured, so one knows where to look for the truncation marker resp. where to expect it. Having the truncation marker in a different column for each displayed filename makes it even harder to grasp the file list (in my opinion).
Performance: The suggested requirement would make it necessary to compare filenames in the list. While it is probably not necessary to compare each filename to each other (which would increase the complexity for displaying the file list from linear to quadratic), at least each neighbouring files or each three or each n neighbours (n <= number of displayed lines) must be compared, which increases the time for displaying a file list n-fold.
Implementation: As mentioned in the previous point, filenames in the list to display must be compared, so the whole function displaying files must probably be re-factored pretty heavily. I am quite happy that the change for the globally fixed percentage is quite small (even if it is bigger than I hoped for).

IMO this implementation (with a fixed percentage) does not offer enough benefits over the current version.

Well, I tend to disagree. In my experience resp. in my filesystems most long filenames differ in the beginning or much more often in the end, especially the file ending, e.g. .jpeg vs. .pdf, or .h vs. .c etc. I never had difficulties with the Midnight Commander (regarding this issue), but so far with lf, that's why I proposed this feature, which solves my problem.

As the proposed extension has not so much benefits (in my opinion), especially with the changes to the code needed, I do not see it as part of this Pull-Request, but maybe for another one (if someone volunteers to implement it and the maintainer is ok with it).

gokcehan · 2022-11-28T16:56:14Z

@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:

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).

raslop · 2022-11-28T20:47:08Z

but there is already a lot of complexity with truncation due to having multiple options for configuration (e.g. info, number)

Well, I think the complexity with truncation is not too high as of now. And the added complexity by my patch is also reasonable (in my opinion, I would even say it simplifies one or two things in ui.go). It worked well in my tests yesterday, even with fileinfo enabled, though I didn't encounter the issue shown.

I was able to reproduce the mentioned issues and fixed them, see FIXUP commits (to be squashed before merge).

I also don't see much benefit in this feature that will make the additional complexity acceptable.

As said before, I believe the additional complexity acceptable.
And I think making it possible to see the end of truncated files is useful.

For many of the usecases I can image there are other ways to handle (i.e. switching to single column when filenames are long).

Right, this is not useful for every use-case and especially not for very narrow windows or extremely long filename. In such cases I indeed switch to a single-column view. But anyway, in some use-cases this is really helpful and save some keypresses to switch to a one-column view.

gokcehan · 2022-11-28T21:15:50Z

@raslop Here's an example with the latest code:

This should be enough to argue that the complexity isn't worth it.

raslop · 2022-11-29T19:53:19Z

This should be enough to argue that the complexity isn't worth it.

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.
I don't think so, at least not completely. It just tells that I haven't understood this part of the code sufficiently well.

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.

gokcehan · 2022-11-29T22:08:29Z

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.

raslop · 2022-12-04T21:44:56Z

Preliminary report, mainly just FYI.

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.

I don't think so. Altogether I only spent a few hours on it, and the vast majority of it was to find out that runeSliceWidthRange() is buggy and trying to debug and fix it (and learn some needed basics about go, e.g. what runes are). I also looked only very narrowly on the second part of printDir() in ui.go and totally missed that the number and relativenumber options are relevant (as I don't use them), which is one main reason why my code needed several iterations.

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.

Ah, ok, now I get it (I think). There was obviously a misunderstanding. When reading "complexity" I though about the complexity of the problem or algorithm (which is quite low here). But you mean the "complexity" of the source code, which closely relates to maintainability and readability of the source code. I agree that the readability could be better, e.g. the repeated calculations of widths with similar meaning in different places is hard to grasp (e.g. win.w-3-len(info)-lnwidth with different constants 2, 3 and 4). Also some of the short identifier names would benefit from being more expressive. And yes, that's another reason main why my code needed the past iterations.

[exponential number of test cases]

I agree, that systematic testing is necessary, e.g. to avoid regressions. In absence of automatic test-tools (I couldn't find any) I only did some manual tests with different values for the options that I thought have influence on my change. I obviously missed some options. From your comment it seems that you also did such tests manually in the past. The large number of test-cases calls for automation. So I wrote a little script that does mostly automated regression tests for one version of lf against a reference version, while permuting all necessary parameters. It employs tmux to control the window width and capture the output of lf after setting lf options with lf -remote "send set ...". The captured output of both lf-instances is then compared using diff. This script showed the differences pointed out by you. If you like, I can share the script.

I have pushed another fix-up commit and used the regression-test-script to verify the absence of further regressions regarding the following options: window-width (via tmux), ratios, preview, info, infotimefmt(new|old), number, relativenumber, dircounts. Anything relevant still missing?

I still see 1846 differences out of 11264 test-cases, all with the same pattern. The reference version (i.e. trunk-version of lf) sometimes truncates the filename to fit the available space, but doesn't append the truncation character. See below screenshot, which shows the diff of this-branch-lf vs. trunk-lf, e.g. the filename complete_test.go (red branch-lf, green trunk-lf). I consider this a fix in this branch.

Regarding runeSliceWidthRange(): If you consider these changes too intrusive, dangerous or risky, I will revert them and add new function, which returns the last n runes of a rune-array.

While there are not real regressions, the actual feature I'm trying to implement still has a bug unfortunately.
I'll come back, once I have fixed that.

As I'm not interested in this feature myself, unfortunately I will not be available for testing these things.

Does that mean, you won't have another look at it, until more people are interested in this?
Especially now, that I can do quite extensive regression-tests?

gokcehan · 2022-12-05T00:17:29Z

@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 · 2023-05-28T12:18:53Z

misc.go

@@ -47,7 +50,7 @@ func runeSliceWidthRange(rs []rune, beg, end int) []rune {
 		}
 		curr += w
 	}
-	return nil
+	return rs[b:curr]


This doesn't look right - curr is a cell index (unlike b which is a rune index) so it doesn't make sense to use curr here for indexing rs. I agree the current implementation doesn't work for a few edge cases, and have suggested an alternate implementation in #1270, but I don't know if it will get accepted or not.

Ok.

I've tried your alternate implementation from #1270 and it works fine for the purpose of this PR too (i.e. properly returns the last part of a rune slice and looks good with different values of fnametruncpct and my ui regression test script shows no regressions).

So I would like your alternate implementation to be merged.

raslop · 2023-05-28T15:51:00Z

After a long time I worked again on this PR.

I have rebased and force-pushed the the branch.

Some of the bugs mentioned above have already been fixed on the master branch, thus this change became easier.

The mentioned regression-test-script (not part of this PR) tested ~117k configuration combinations and found no differences.

However, I just noticed #1270 and decided to make my PR a draft again, until the runeSliceWidthRange stuff from #1270 is discussed.

joelim-work · 2023-05-29T02:45:19Z

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.

raslop · 2023-05-29T12:22:47Z

I think it's best to split the task into two parts, one to refactor the code and another to actually implement the feature.

Sounds reasonable.

I have already submitted a PR for the refactor part, see #1272.

Thanks, that's great. I had a short (not too deep) look at it, and it looks good to me (and vaguely familiar).

Hopefully once that gets merged, you can then implement your feature on top of it.

Yes, my feature will then only change a few lines 😃

Apologies for intruding upon your PR, but this cleanup was something I wanted to do regardless.

No need for apologies. I'm happy someone with more knowledge about lf and especially go did/does this cleanup.

raslop · 2023-06-08T18:53:28Z

Now that #1272 and #1273 are merged (thanks @joelim-work and @gokcehan), I'll try again with my feature.

I've rebased to the current master and force-pushed my branch.

I figured that runeSliceWidthRange seems not suitable for my use case to return the last runes of a rune-slice that fit into a given width. So I created a new function (runeSliceWidthLastRange) for that in misc.go (I'm not sure if the name matches the name pattern).
The rest of the patch consists of the (unchanged) boilerplate for a new option and the now quite small diff for my feature in ui.go.

[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).]

joelim-work · 2023-06-09T05:36:32Z

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 `max_width` space.
+func runeSliceWidthLastRange(rs []rune, max_width int) []rune {


The implementation for this function looks fine, but please add unit tests in misc_test.go, including tests with Chinese characters like 世界. You can use the same inputs as the unit tests for runeSliceWidthRange.

Sure, test cases are now added.

joelim-work · 2023-06-09T05:38:27Z

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 `max_width` space.
+func runeSliceWidthLastRange(rs []rune, max_width int) []rune {
+	last_width := 0


Regarding naming conventions, the recommendation for Go is to use camelcase, see https://go.dev/doc/effective_go#mixed-caps

Suggested change

last_width := 0

lastWidth := 0

Ok, thanks. Is now changed.

joelim-work · 2023-06-09T05:51:19Z

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)
+			var truncatePos int = (maxFilenameWidth - 1) * gOpts.fnametruncpct / 100


Minor: I'm not sure if you had a reason for not using short declarations (i.e. := syntax), but from what I can see most of the code uses this so it would be better to follow it:

Suggested change

var truncatePos int = (maxFilenameWidth - 1) * gOpts.fnametruncpct / 100

truncatePos := (maxFilenameWidth - 1) * gOpts.fnametruncpct / 100

Well, I want truncatePos to be an integer instead of a float. I vaguely remember that this "expclicit cast" (or what ever this is called in go) was necessary. But now I can't reproduce this anymore and it seems to make no difference. So I have changed it to the short declaration.

joelim-work · 2023-06-09T05:53:47Z

@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 · 2023-06-09T11:54:31Z

[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.

As of now, lf truncates always at the end. So for long filenames one can not see the end (especially not the file ending, like .jpeg or .pdf). [MidnightCommander](https://github.com/MidnightCommander/mc/) truncates too long filenames in the middle, so it's possible to see the beginning and the end of the too long filename. This commit allows to specify a flexible point for the truncation in percent of the available space.

raslop

@gokcehan , @joelim-work Thanks for your time, comments and suggestions.

I have updated the branch (rebased and force-pushed) and done the changes asked for (rename option to truncatepct, add test cases for new function, adhere to naming convention).

If there's still anything missing or not right yet, please let me know.

raslop · 2023-06-09T20:46:14Z

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 `max_width` space.
+func runeSliceWidthLastRange(rs []rune, max_width int) []rune {
+	last_width := 0


Ok, thanks. Is now changed.

raslop · 2023-06-09T20:49:22Z

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)
+			var truncatePos int = (maxFilenameWidth - 1) * gOpts.fnametruncpct / 100


Well, I want truncatePos to be an integer instead of a float. I vaguely remember that this "expclicit cast" (or what ever this is called in go) was necessary. But now I can't reproduce this anymore and it seems to make no difference. So I have changed it to the short declaration.

raslop · 2023-06-09T20:49:53Z

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 `max_width` space.
+func runeSliceWidthLastRange(rs []rune, max_width int) []rune {


Sure, test cases are now added.

ilyagr

You've got plenty of reviewers already, so I only glanced at this and had a couple of minor comments. Thanks for the feature, I'll try it out and will likely enjoy it!

ilyagr · 2023-06-09T22:46:15Z

doc.go

+
+  - `set truncatepct 0`   -> "~ng-filename-truncated"
+
+The default of 100 means no change in behaviour to before the introduction of


There's no need to say how this compares to previous versions in the documentation. You can put it into the PR/commit description if you haven't already.

Right. I've removed the sentence and put it in the commit message (of the next additional commit).

ilyagr · 2023-06-09T22:47:30Z

eval.go

+			return
+		}
+		if n < 0 || n > 100 {
+			app.ui.echoerr("truncatepct: must be between 0 and 100 (both inclusive)")


Very minor: It would be nice to include ", got %d" at the end of this error message to print the value of n (and also change it to echoerrf).

Right, thanks. Changed.

joelim-work · 2023-06-10T01:53:58Z

misc_test.go

+		{[]rune{'世', '界', '世', '界'}, 8, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 9, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 10, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 11, []rune{'世', '界', '世', '界'}},


Very minor: This slice only has a width of 8, perhaps checking for a value of 11 is not necessary, and you can probably get rid of the test case for 10 as well.

Yes, you are right the test cases with 10 and 11 are redundant (at least for the current implementation). This is just some going a little bit further, just in case.
Anyway, I've removed them.

joelim-work · 2023-06-10T01:54:46Z

misc.go

+	lastWidth := 0
+	for i := len(rs) - 1; i >= 0; i-- {
+		w := runewidth.RuneWidth(rs[i])
+		if lastWidth+w > max_width {


This should be camelcase too, apologies I missed it last time.

Suggested change

if lastWidth+w > max_width {

if lastWidth+w > maxWidth {

I missed that one too. Now it's also changed.

change in lf behaviour.

raslop

Thanks.
I've pushed some additional to address the new comments.

raslop · 2023-06-10T08:07:03Z

doc.go

+
+  - `set truncatepct 0`   -> "~ng-filename-truncated"
+
+The default of 100 means no change in behaviour to before the introduction of


Right. I've removed the sentence and put it in the commit message (of the next additional commit).

raslop · 2023-06-10T08:08:51Z

eval.go

+			return
+		}
+		if n < 0 || n > 100 {
+			app.ui.echoerr("truncatepct: must be between 0 and 100 (both inclusive)")


Right, thanks. Changed.

raslop · 2023-06-10T08:09:14Z

misc.go

+	lastWidth := 0
+	for i := len(rs) - 1; i >= 0; i-- {
+		w := runewidth.RuneWidth(rs[i])
+		if lastWidth+w > max_width {


I missed that one too. Now it's also changed.

raslop · 2023-06-10T08:14:21Z

misc_test.go

+		{[]rune{'世', '界', '世', '界'}, 8, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 9, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 10, []rune{'世', '界', '世', '界'}},
+		{[]rune{'世', '界', '世', '界'}, 11, []rune{'世', '界', '世', '界'}},


Yes, you are right the test cases with 10 and 11 are redundant (at least for the current implementation). This is just some going a little bit further, just in case.
Anyway, I've removed them.

joelim-work

I have a couple more minor comments, apologies for missing them earlier. But I think this should be the last of them, and if you fix these then I would be willing to approve your PR.

joelim-work · 2023-06-10T15:13:03Z

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
+			var lastPart []rune = runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)


runeSliceWidthLastRange already returns []rune so you should just be able to write this:

Suggested change

var lastPart []rune = runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)

lastPart := runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)

Ok, yes it works, thanks.
Changed accordingly.

joelim-work · 2023-06-10T15:13:48Z

ui.go

 			filename = append(filename, []rune(gOpts.truncatechar)...)
+			filename = append(filename, []rune(lastPart)...)


Similarly lastPart is already a []rune so you don't need a cast here:

Suggested change

filename = append(filename, []rune(lastPart)...)

filename = append(filename, lastPart...)

Works too, thanks.
Also changed.

raslop

Thanks for the hints. I've pushed another commit to address them.

raslop · 2023-06-10T19:14:47Z

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
+			var lastPart []rune = runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)


Ok, yes it works, thanks.
Changed accordingly.

raslop · 2023-06-10T19:15:20Z

ui.go

 			filename = append(filename, []rune(gOpts.truncatechar)...)
+			filename = append(filename, []rune(lastPart)...)


Works too, thanks.
Also changed.

ilyagr · 2023-06-11T05:46:40Z

doc.go

+
+  - `set truncatepct 0`   -> "~ng-filename-truncated"
+
+.


Thanks for the fix! One last nit: could you remove this period and make sure there's one blank line before the next item?

Ok, well.
After removing the "100 does not change the behaviour" line after the examples list, I saw that go fmt would re-format the next line with waitmsg (which I don't want to touch in this change) unnecessarily (i.e. convert the leading tab to space). That's why I introduced the line with the single dot, to work around this unexpected formatting behaviour.
However, I tried other things and found that removing the spaces before the list items works too (i.e. go fmt doesn't touch the next item with waitmsg)
See the additional commit.

This was there to avoid that `go fmt` touches the next item with `waitmsg`. However, removing the spaces before the example list items has the same effect.

raslop

I've added another commit to address the "line with single dot" in doc.go issue.

raslop · 2023-06-11T10:17:29Z

doc.go

+
+  - `set truncatepct 0`   -> "~ng-filename-truncated"
+
+.


Ok, well.
After removing the "100 does not change the behaviour" line after the examples list, I saw that go fmt would re-format the next line with waitmsg (which I don't want to touch in this change) unnecessarily (i.e. convert the leading tab to space). That's why I introduced the line with the single dot, to work around this unexpected formatting behaviour.
However, I tried other things and found that removing the spaces before the list items works too (i.e. go fmt doesn't touch the next item with waitmsg)
See the additional commit.

gokcehan · 2023-07-03T09:48:46Z

Thanks @raslop for the patch and @joelim-work and @ilyagr for the reviews.

raslop · 2023-07-03T19:11:44Z

Thanks, all.

raslop marked this pull request as draft November 27, 2022 11:30

raslop marked this pull request as ready for review November 27, 2022 12:05

joelim-work mentioned this pull request May 28, 2023

Fix typo in misc tests #1270

Merged

joelim-work reviewed May 28, 2023

View reviewed changes

raslop marked this pull request as draft May 28, 2023 15:51

raslop marked this pull request as ready for review June 8, 2023 18:53

joelim-work reviewed Jun 9, 2023

View reviewed changes

raslop commented Jun 9, 2023

View reviewed changes

ilyagr reviewed Jun 9, 2023

View reviewed changes

joelim-work reviewed Jun 10, 2023

View reviewed changes

raslop added 4 commits June 10, 2023 10:15

The default value of 100 for the new truncatepct option means no

e5e3bcd

change in lf behaviour.

More adhering to naming convention.

3b3ac9b

Improve error message for invalid truncatepct values.

6f94846

Remove redundant test cases.

a9fd7f3

raslop commented Jun 10, 2023

View reviewed changes

FIXUP Add dummy empty line to make go fmt happy.

2422a8c

joelim-work reviewed Jun 10, 2023

View reviewed changes

Avoid explicit "cast" to rune array.

0408539

raslop commented Jun 10, 2023

View reviewed changes

joelim-work approved these changes Jun 11, 2023

View reviewed changes

ilyagr reviewed Jun 11, 2023

View reviewed changes

fix(doc.go): remove line with single dot

ed45edc

This was there to avoid that `go fmt` touches the next item with `waitmsg`. However, removing the spaces before the example list items has the same effect.

raslop commented Jun 11, 2023

View reviewed changes

gokcehan merged commit b1a8519 into gokcehan:master Jul 3, 2023

gokcehan mentioned this pull request Sep 17, 2023

r31 changelog #1407

Closed

	var truncatePos int = (maxFilenameWidth - 1) * gOpts.fnametruncpct / 100
	truncatePos := (maxFilenameWidth - 1) * gOpts.fnametruncpct / 100


		- `set truncatepct 0` -> "~ng-filename-truncated"

		The default of 100 means no change in behaviour to before the introduction of

	var lastPart []rune = runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)
	lastPart := runeSliceWidthLastRange(filename, maxFilenameWidth-truncatePos-1)

		filename = append(filename, []rune(gOpts.truncatechar)...)
		filename = append(filename, []rune(lastPart)...)

	filename = append(filename, []rune(lastPart)...)
	filename = append(filename, lastPart...)

Flexible filename truncation #1029

Flexible filename truncation #1029

Conversation

raslop commented Nov 27, 2022

laktak commented Nov 27, 2022

raslop commented Nov 27, 2022 • edited Loading

gokcehan commented Nov 28, 2022

raslop commented Nov 28, 2022

gokcehan commented Nov 28, 2022

raslop commented Nov 29, 2022

gokcehan commented Nov 29, 2022

raslop commented Dec 4, 2022

gokcehan commented Dec 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raslop commented May 28, 2023

joelim-work commented May 29, 2023

raslop commented May 29, 2023

raslop commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelim-work commented Jun 9, 2023

gokcehan commented Jun 9, 2023

raslop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilyagr left a comment

Choose a reason for hiding this comment

ilyagr Jun 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raslop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelim-work left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raslop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raslop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gokcehan commented Jul 3, 2023

raslop commented Jul 3, 2023

raslop commented Nov 27, 2022 •

edited

Loading

ilyagr Jun 9, 2023 •

edited

Loading