Repository: gokcehan/lf
Pull request #1273 (2023-05-29 03:21:41)

Fix `runeSliceWidthRange` implementation
Follow up of #1270 

I have decided to submit the proposed changes to the `runeSliceWidthRange` function, and added some more unit tests to cover scenarios which aren't handled by the current implementation. In my opinion, it is important to ensure the internal API is robust, otherwise it can become difficult to add new changes.

Thanks to @raslop for having a look at this beforehand.

ilyagr: Looks great, but I've only glanced at it so far. I'd like to try this out and think about potential bugs some more, perhaps learn something about runes and Unicode. If somebody else can review it faster than me, feel free!

Patch:
diff --git a/complete.go b/complete.go
index d2e04dc..125eeac 100644
--- a/complete.go
+++ b/complete.go
@@ -16,6 +16,7 @@ var (
 		"cmap",
 		"cmaps",
 		"cmd",
+		"cmds",
 		"quit",
 		"up",
 		"half-up",
diff --git a/doc.go b/doc.go
index 096cf5f..e7c15e4 100644
--- a/doc.go
+++ b/doc.go
@@ -81,6 +81,7 @@ The following commands are provided by lf:
 	tag-toggle               (default 't')
 	maps
 	cmaps
+	cmds
 	jumps
 
 The following command line commands are provided by lf:
@@ -620,6 +621,10 @@ Capitalize/uppercase/lowercase the current word and jump to the next word.
 
 List all key mappings in normal mode or command-line editing mode.
 
+	cmds
+
+List all custom commands defined using the `cmd` command
+
 	jumps
 
 List the contents of the jump list, in order of the most recently visited locations.
diff --git a/docstring.go b/docstring.go
index f716670..1c1c5e2 100644
--- a/docstring.go
+++ b/docstring.go
@@ -84,6 +84,7 @@ The following commands are provided by lf:
     tag-toggle               (default 't')
     maps
     cmaps
+    cmds
     jumps
 
 The following command line commands are provided by lf:
@@ -648,6 +649,10 @@ Capitalize/uppercase/lowercase the current word and jump to the next word.
 
 List all key mappings in normal mode or command-line editing mode.
 
+    cmds
+
+List all custom commands defined using the 'cmd' command
+
     jumps
 
 List the contents of the jump list, in order of the most recently visited
diff --git a/etc/icons.example b/etc/icons.example
index 8039c93..404f9e5 100644
--- a/etc/icons.example
+++ b/etc/icons.example
@@ -87,7 +87,7 @@ fi             # FILE
 *.cc            
 *.cp            
 *.c             
-*.cs            
+*.cs            󰌛
 *.h             
 *.hh            
 *.hpp           
@@ -152,10 +152,10 @@ fi             # FILE
 *.elm           
 *.swift         
 *.xcplayground  
-*.tex           ﭨ
-*.r             ﳒ
-*.rproj         鉶
-*.sol           ﲹ
+*.tex           󰙩
+*.r             󰟔
+*.rproj         󰗆
+*.sol           󰡪
 *.pem           
 
 # file names (vim-devicons) (case-insensitive not supported in lf)
@@ -193,7 +193,7 @@ fi             # FILE
 *gemfile                
 *makefile               
 *cmakelists.txt         
-*robots.txt             ﮧ
+*robots.txt             󰚩
 
 # file names (case-sensitive adaptations)
 *Gruntfile.coffee       
diff --git a/eval.go b/eval.go
index 0ddf107..81b9267 100644
--- a/eval.go
+++ b/eval.go
@@ -2244,64 +2244,60 @@ func (e *callExpr) eval(app *app, args []string) {
 			}
 		case "rename: ":
 			app.ui.cmdPrefix = ""
-			if curr, err := app.nav.currFile(); err != nil {
-				app.ui.echoerrf("rename: %s", err)
-			} else {
-				wd, err := os.Getwd()
-				if err != nil {
-					log.Printf("getting current directory: %s", err)
-					return
-				}
-
-				oldPath := filepath.Join(wd, curr.Name())
-
-				newPath := filepath.Clean(replaceTilde(s))
-				if !filepath.IsAbs(newPath) {
-					newPath = filepath.Join(wd, newPath)
-				}
 
-				if oldPath == newPath {
-					return
-				}
+			curr, err := app.nav.currFile()
+			if err != nil {
+				app.ui.echoerrf("rename: %s", err)
+				return
+			}
+			wd, err := os.Getwd()
+			if err != nil {
+				log.Printf("getting current directory: %s", err)
+				return
+			}
 
-				app.nav.renameOldPath = oldPath
-				app.nav.renameNewPath = newPath
+			oldPath := filepath.Join(wd, curr.Name())
+			newPath := filepath.Clean(replaceTilde(s))
+			if !filepath.IsAbs(newPath) {
+				newPath = filepath.Join(wd, newPath)
+			}
+			if oldPath == newPath {
+				return
+			}
+			app.nav.renameOldPath = oldPath
+			app.nav.renameNewPath = newPath
 
-				newDir := filepath.Dir(newPath)
-				if _, err := os.Stat(newDir); os.IsNotExist(err) {
-					app.ui.cmdPrefix = "create '" + newDir + "' ? [y/N] "
-					return
-				}
+			newDir := filepath.Dir(newPath)
+			if _, err := os.Stat(newDir); os.IsNotExist(err) {
+				app.ui.cmdPrefix = "create '" + newDir + "' ? [y/N] "
+				return
+			}
 
-				oldStat, err := os.Lstat(oldPath)
-				if err != nil {
-					app.ui.echoerrf("rename: %s", err)
-					return
-				}
+			oldStat, err := os.Lstat(oldPath)
+			if err != nil {
+				app.ui.echoerrf("rename: %s", err)
+				return
+			}
+			if newStat, err := os.Lstat(newPath); !os.IsNotExist(err) && !os.SameFile(oldStat, newStat) {
+				app.ui.cmdPrefix = "replace '" + newPath + "' ? [y/N] "
+				return
+			}
 
-				if newStat, err := os.Lstat(newPath); !os.IsNotExist(err) && !os.SameFile(oldStat, newStat) {
-					app.ui.cmdPrefix = "replace '" + newPath + "' ? [y/N] "
-					return
-				}
+			if err := app.nav.rename(); err != nil {
+				app.ui.echoerrf("rename: %s", err)
+				return
+			}
 
-				if err := app.nav.rename(); err != nil {
+			if gSingleMode {
+				app.nav.renew()
+			} else {
+				if err := remote("send load"); err != nil {
 					app.ui.echoerrf("rename: %s", err)
 					return
 				}
-
-				if gSingleMode {
-					app.nav.renew()
-					app.ui.loadFile(app, true)
-				} else {
-					if err := remote("send load"); err != nil {
-						app.ui.echoerrf("rename: %s", err)
-						return
-					}
-				}
-
-				app.ui.loadFile(app, true)
-				app.ui.loadFileInfo(app.nav)
 			}
+			app.ui.loadFile(app, true)
+			app.ui.loadFileInfo(app.nav)
 		default:
 			log.Printf("entering unknown execution prefix: %q", app.ui.cmdPrefix)
 		}
@@ -2347,9 +2343,10 @@ func (e *callExpr) eval(app *app, args []string) {
 			switch app.ui.cmdPrefix {
 			case "!", "$", "%", "&":
 				app.ui.cmdPrefix = ":"
-			case ">", "rename: ":
+			case ">", "rename: ", "filter: ":
 				// Don't mess with programs waiting for input.
-				// Exiting on backspace is also inconvenient for renames since the text field starts out nonempty.
+				// Exiting on backspace is also inconvenient for 'rename' and 'filter',
+				// since the text field can start out nonempty.
 			default:
 				normal(app)
 			}
@@ -2525,6 +2522,8 @@ func (e *callExpr) eval(app *app, args []string) {
 
 		app.ui.cmdAccLeft = acc
 		update(app)
+	case "cmds":
+		app.runPagerOnText(listCmds())
 	case "maps":
 		app.runPagerOnText(listBinds(gOpts.keys))
 	case "cmaps":
diff --git a/lf.1 b/lf.1
index 6de2b89..e749bfe 100644
--- a/lf.1
+++ b/lf.1
@@ -96,6 +96,7 @@ The following commands are provided by lf:
     tag-toggle               (default 't')
     maps
     cmaps
+    cmds
     jumps
 .EE
 .PP
@@ -764,6 +765,12 @@ Capitalize/uppercase/lowercase the current word and jump to the next word.
 .PP
 List all key mappings in normal mode or command-line editing mode.
 .PP
+.EX
+    cmds
+.EE
+.PP
+List all custom commands defined using the `cmd` command
+.PP
 .EX
     jumps
 .EE
diff --git a/misc.go b/misc.go
index ea33664..b0303ea 100644
--- a/misc.go
+++ b/misc.go
@@ -31,23 +31,26 @@ func runeSliceWidth(rs []rune) int {
 }
 
 func runeSliceWidthRange(rs []rune, beg, end int) []rune {
+	if beg == end {
+		return []rune{}
+	}
+
 	curr := 0
 	b := 0
+	foundb := false
 	for i, r := range rs {
 		w := runewidth.RuneWidth(r)
-		switch {
-		case curr == beg:
+		if curr >= beg && !foundb {
 			b = i
-		case curr < beg && curr+w > beg:
-			b = i + 1
-		case curr == end:
+			foundb = true
+		}
+		if curr == end || curr+w > end {
 			return rs[b:i]
-		case curr > end:
-			return rs[b : i-1]
 		}
 		curr += w
 	}
-	return nil
+
+	return rs[b:]
 }
 
 // This function is used to escape whitespaces and special characters with
diff --git a/ui.go b/ui.go
index 4c01391..aed1153 100644
--- a/ui.go
+++ b/ui.go
@@ -1052,7 +1052,7 @@ func findBinds(keys map[string]expr, prefix string) (binds map[string]expr, ok b
 	return
 }
 
-func listBinds(binds map[string]expr) *bytes.Buffer {
+func listExprMap(binds map[string]expr, title string) *bytes.Buffer {
 	t := new(tabwriter.Writer)
 	b := new(bytes.Buffer)
 
@@ -1063,7 +1063,7 @@ func listBinds(binds map[string]expr) *bytes.Buffer {
 	sort.Strings(keys)
 
 	t.Init(b, 0, gOpts.tabstop, 2, '\t', 0)
-	fmt.Fprintln(t, "keys\tcommand")
+	fmt.Fprintf(t, "%s\tcommand\n", title)
 	for _, k := range keys {
 		fmt.Fprintf(t, "%s\t%v\n", k, binds[k])
 	}
@@ -1072,6 +1072,14 @@ func listBinds(binds map[string]expr) *bytes.Buffer {
 	return b
 }
 
+func listBinds(binds map[string]expr) *bytes.Buffer {
+	return listExprMap(binds, "keys")
+}
+
+func listCmds() *bytes.Buffer {
+	return listExprMap(gOpts.cmds, "name")
+}
+
 func listJumps(jumps []string, ind int) *bytes.Buffer {
 	t := new(tabwriter.Writer)
 	b := new(bytes.Buffer)


Test patch:
diff --git a/misc_test.go b/misc_test.go
index 3568fa2..aa6f6df 100644
--- a/misc_test.go
+++ b/misc_test.go
@@ -55,21 +55,30 @@ func TestRuneSliceWidthRange(t *testing.T) {
 		end int
 		exp []rune
 	}{
+		{[]rune{}, 0, 0, []rune{}},
 		{[]rune{'a', 'b', 'c', 'd'}, 1, 3, []rune{'b', 'c'}},
 		{[]rune{'a', 'ı', 'b', 'ş'}, 1, 3, []rune{'ı', 'b'}},
 		{[]rune{'世', '界', '世', '界'}, 2, 6, []rune{'界', '世'}},
 		{[]rune{'世', '界', '世', '界'}, 3, 6, []rune{'世'}},
 		{[]rune{'世', '界', '世', '界'}, 2, 5, []rune{'界'}},
 		{[]rune{'世', '界', '世', '界'}, 3, 5, []rune{}},
+		{[]rune{'世', '界', '世', '界'}, 4, 4, []rune{}},
+		{[]rune{'世', '界', '世', '界'}, 5, 5, []rune{}},
+		{[]rune{'世', '界', '世', '界'}, 4, 7, []rune{'世'}},
+		{[]rune{'世', '界', '世', '界'}, 4, 8, []rune{'世', '界'}},
 		{[]rune{'世', 'a', '界', 'ı'}, 2, 5, []rune{'a', '界'}},
 		{[]rune{'世', 'a', '界', 'ı'}, 2, 4, []rune{'a'}},
 		{[]rune{'世', 'a', '界', 'ı'}, 3, 5, []rune{'界'}},
 		{[]rune{'世', 'a', '界', 'ı'}, 3, 4, []rune{}},
+		{[]rune{'世', 'a', '界', 'ı'}, 3, 3, []rune{}},
+		{[]rune{'世', 'a', '界', 'ı'}, 4, 4, []rune{}},
+		{[]rune{'世', 'a', '界', 'ı'}, 4, 6, []rune{'ı'}},
+		{[]rune{'世', 'a', '界', 'ı'}, 5, 6, []rune{'ı'}},
 	}
 
 	for _, test := range tests {
 		if got := runeSliceWidthRange(test.rs, test.beg, test.end); !reflect.DeepEqual(got, test.exp) {
-			t.Errorf("at input '%v' expected '%v' but got '%v'", test.rs, test.rs, got)
+			t.Errorf("at input '%v' expected '%v' but got '%v'", test.rs, test.exp, got)
 		}
 	}
 }