Minor refactoring

- Slightly more efficient processing of Options
- Do not return reference type arguments that are mutated inside the
  function
- Use util.Constrain function when appropriate
This commit is contained in:
Junegunn Choi 2016-02-18 01:46:18 +09:00
parent 45108ddd53
commit e72a360337
3 changed files with 88 additions and 48 deletions

View File

@ -163,7 +163,7 @@ func defaultOptions() *Options {
Filter: nil,
ToggleSort: false,
Expect: make(map[int]string),
Keymap: defaultKeymap(),
Keymap: make(map[int]actionType),
Execmap: make(map[int]string),
PrintQuery: false,
ReadZero: false,
@ -484,7 +484,7 @@ const (
escapedComma = 1
)
func parseKeymap(keymap map[int]actionType, execmap map[int]string, toggleSort bool, str string) (map[int]actionType, map[int]string, bool) {
func parseKeymap(keymap map[int]actionType, execmap map[int]string, str string) {
if executeRegexp == nil {
// Backreferences are not supported.
// "~!@#$%^&*;/|".each_char.map { |c| Regexp.escape(c) }.map { |c| "#{c}[^#{c}]*#{c}" }.join('|')
@ -592,7 +592,6 @@ func parseKeymap(keymap map[int]actionType, execmap map[int]string, toggleSort b
keymap[key] = actNextHistory
case "toggle-sort":
keymap[key] = actToggleSort
toggleSort = true
default:
if isExecuteAction(actLower) {
var offset int
@ -613,7 +612,6 @@ func parseKeymap(keymap map[int]actionType, execmap map[int]string, toggleSort b
}
}
}
return keymap, execmap, toggleSort
}
func isExecuteAction(str string) bool {
@ -635,13 +633,12 @@ func isExecuteAction(str string) bool {
return false
}
func checkToggleSort(keymap map[int]actionType, str string) map[int]actionType {
func parseToggleSort(keymap map[int]actionType, str string) {
keys := parseKeyChords(str, "key name required")
if len(keys) != 1 {
errorExit("multiple keys specified")
}
keymap[firstKey(keys)] = actToggleSort
return keymap
}
func strLines(str string) []string {
@ -691,7 +688,6 @@ func parseMargin(margin string) [4]string {
}
func parseOptions(opts *Options, allArgs []string) {
keymap := make(map[int]actionType)
var historyMax int
if opts.History == nil {
historyMax = defaultHistoryMax
@ -741,8 +737,7 @@ func parseOptions(opts *Options, allArgs []string) {
case "--tiebreak":
opts.Criteria = parseTiebreak(nextString(allArgs, &i, "sort criterion required"))
case "--bind":
keymap, opts.Execmap, opts.ToggleSort =
parseKeymap(keymap, opts.Execmap, opts.ToggleSort, nextString(allArgs, &i, "bind expression required"))
parseKeymap(opts.Keymap, opts.Execmap, nextString(allArgs, &i, "bind expression required"))
case "--color":
spec := optionalNextString(allArgs, &i)
if len(spec) == 0 {
@ -751,8 +746,7 @@ func parseOptions(opts *Options, allArgs []string) {
opts.Theme = parseTheme(opts.Theme, spec)
}
case "--toggle-sort":
keymap = checkToggleSort(keymap, nextString(allArgs, &i, "key name required"))
opts.ToggleSort = true
parseToggleSort(opts.Keymap, nextString(allArgs, &i, "key name required"))
case "-d", "--delimiter":
opts.Delimiter = delimiterRegexp(nextString(allArgs, &i, "delimiter required"))
case "-n", "--nth":
@ -869,8 +863,7 @@ func parseOptions(opts *Options, allArgs []string) {
} else if match, _ := optString(arg, "-s", "--sort="); match {
opts.Sort = 1 // Don't care
} else if match, value := optString(arg, "--toggle-sort="); match {
keymap = checkToggleSort(keymap, value)
opts.ToggleSort = true
parseToggleSort(opts.Keymap, value)
} else if match, value := optString(arg, "--expect="); match {
opts.Expect = parseKeyChords(value, "key names required")
} else if match, value := optString(arg, "--tiebreak="); match {
@ -878,8 +871,7 @@ func parseOptions(opts *Options, allArgs []string) {
} else if match, value := optString(arg, "--color="); match {
opts.Theme = parseTheme(opts.Theme, value)
} else if match, value := optString(arg, "--bind="); match {
keymap, opts.Execmap, opts.ToggleSort =
parseKeymap(keymap, opts.Execmap, opts.ToggleSort, value)
parseKeymap(opts.Keymap, opts.Execmap, value)
} else if match, value := optString(arg, "--history="); match {
setHistory(value)
} else if match, value := optString(arg, "--history-size="); match {
@ -905,21 +897,28 @@ func parseOptions(opts *Options, allArgs []string) {
if opts.Tabstop < 1 {
errorExit("tab stop must be a positive integer")
}
}
// Change default actions for CTRL-N / CTRL-P when --history is used
func postProcessOptions(opts *Options) {
// Default actions for CTRL-N / CTRL-P when --history is set
if opts.History != nil {
if _, prs := keymap[curses.CtrlP]; !prs {
keymap[curses.CtrlP] = actPreviousHistory
if _, prs := opts.Keymap[curses.CtrlP]; !prs {
opts.Keymap[curses.CtrlP] = actPreviousHistory
}
if _, prs := keymap[curses.CtrlN]; !prs {
keymap[curses.CtrlN] = actNextHistory
if _, prs := opts.Keymap[curses.CtrlN]; !prs {
opts.Keymap[curses.CtrlN] = actNextHistory
}
}
// Override default key bindings
for key, act := range keymap {
opts.Keymap[key] = act
// Extend the default key map
keymap := defaultKeymap()
for key, act := range opts.Keymap {
if act == actToggleSort {
opts.ToggleSort = true
}
keymap[key] = act
}
opts.Keymap = keymap
// If we're not using extended search mode, --nth option becomes irrelevant
// if it contains the whole range
@ -939,9 +938,13 @@ func ParseOptions() *Options {
// Options from Env var
words, _ := shellwords.Parse(os.Getenv("FZF_DEFAULT_OPTS"))
parseOptions(opts, words)
if len(words) > 0 {
parseOptions(opts, words)
}
// Options from command-line arguments
parseOptions(opts, os.Args[1:])
postProcessOptions(opts)
return opts
}

View File

@ -96,6 +96,7 @@ func TestIrrelevantNth(t *testing.T) {
opts := defaultOptions()
words := []string{"--nth", "..", "-x"}
parseOptions(opts, words)
postProcessOptions(opts)
if len(opts.Nth) != 0 {
t.Errorf("nth should be empty: %s", opts.Nth)
}
@ -104,6 +105,7 @@ func TestIrrelevantNth(t *testing.T) {
{
opts := defaultOptions()
parseOptions(opts, words)
postProcessOptions(opts)
if len(opts.Nth) != 0 {
t.Errorf("nth should be empty: %s", opts.Nth)
}
@ -112,6 +114,7 @@ func TestIrrelevantNth(t *testing.T) {
opts := defaultOptions()
words = append(words, "-x")
parseOptions(opts, words)
postProcessOptions(opts)
if len(opts.Nth) != 2 {
t.Errorf("nth should not be empty: %s", opts.Nth)
}
@ -231,15 +234,11 @@ func TestBind(t *testing.T) {
keymap := defaultKeymap()
execmap := make(map[int]string)
check(actBeginningOfLine, keymap[curses.CtrlA])
keymap, execmap, toggleSort :=
parseKeymap(keymap, execmap, false,
"ctrl-a:kill-line,ctrl-b:toggle-sort,c:page-up,alt-z:page-down,"+
"f1:execute(ls {}),f2:execute/echo {}, {}, {}/,f3:execute[echo '({})'],f4:execute;less {};,"+
"alt-a:execute@echo (,),[,],/,:,;,%,{}@,alt-b:execute;echo (,),[,],/,:,@,%,{};"+
",,:abort,::accept,X:execute:\nfoobar,Y:execute(baz)")
if !toggleSort {
t.Errorf("toggleSort not set")
}
parseKeymap(keymap, execmap,
"ctrl-a:kill-line,ctrl-b:toggle-sort,c:page-up,alt-z:page-down,"+
"f1:execute(ls {}),f2:execute/echo {}, {}, {}/,f3:execute[echo '({})'],f4:execute;less {};,"+
"alt-a:execute@echo (,),[,],/,:,;,%,{}@,alt-b:execute;echo (,),[,],/,:,@,%,{};"+
",,:abort,::accept,X:execute:\nfoobar,Y:execute(baz)")
check(actKillLine, keymap[curses.CtrlA])
check(actToggleSort, keymap[curses.CtrlB])
check(actPageUp, keymap[curses.AltZ+'c'])
@ -259,15 +258,11 @@ func TestBind(t *testing.T) {
checkString("\nfoobar,Y:execute(baz)", execmap[curses.AltZ+'X'])
for idx, char := range []rune{'~', '!', '@', '#', '$', '%', '^', '&', '*', '|', ';', '/'} {
keymap, execmap, toggleSort =
parseKeymap(keymap, execmap, false, fmt.Sprintf("%d:execute%cfoobar%c", idx%10, char, char))
parseKeymap(keymap, execmap, fmt.Sprintf("%d:execute%cfoobar%c", idx%10, char, char))
checkString("foobar", execmap[curses.AltZ+int([]rune(fmt.Sprintf("%d", idx%10))[0])])
}
keymap, execmap, toggleSort = parseKeymap(keymap, execmap, false, "f1:abort")
if toggleSort {
t.Errorf("toggleSort set")
}
parseKeymap(keymap, execmap, "f1:abort")
check(actAbort, keymap[curses.F1])
}
@ -328,3 +323,53 @@ func TestParseNilTheme(t *testing.T) {
t.Errorf("color should now be enabled and customized")
}
}
func TestDefaultCtrlNP(t *testing.T) {
check := func(words []string, key int, expected actionType) {
opts := defaultOptions()
parseOptions(opts, words)
postProcessOptions(opts)
if opts.Keymap[key] != expected {
t.Error()
}
}
check([]string{}, curses.CtrlN, actDown)
check([]string{}, curses.CtrlP, actUp)
check([]string{"--bind=ctrl-n:accept"}, curses.CtrlN, actAccept)
check([]string{"--bind=ctrl-p:accept"}, curses.CtrlP, actAccept)
hist := "--history=/tmp/foo"
check([]string{hist}, curses.CtrlN, actNextHistory)
check([]string{hist}, curses.CtrlP, actPreviousHistory)
check([]string{hist, "--bind=ctrl-n:accept"}, curses.CtrlN, actAccept)
check([]string{hist, "--bind=ctrl-n:accept"}, curses.CtrlP, actPreviousHistory)
check([]string{hist, "--bind=ctrl-p:accept"}, curses.CtrlN, actNextHistory)
check([]string{hist, "--bind=ctrl-p:accept"}, curses.CtrlP, actAccept)
}
func TestToggle(t *testing.T) {
optsFor := func(words ...string) *Options {
opts := defaultOptions()
parseOptions(opts, words)
postProcessOptions(opts)
return opts
}
opts := optsFor()
if opts.ToggleSort {
t.Error()
}
opts = optsFor("--bind=a:toggle-sort")
if !opts.ToggleSort {
t.Error()
}
opts = optsFor("--bind=a:toggle-sort", "--bind=a:up")
if opts.ToggleSort {
t.Error()
}
}

View File

@ -1122,15 +1122,7 @@ func (t *Terminal) constrain() {
diffpos := t.cy - t.offset
t.cy = util.Constrain(t.cy, 0, count-1)
if t.cy > t.offset+(height-1) {
// Ceil
t.offset = t.cy - (height - 1)
} else if t.offset > t.cy {
// Floor
t.offset = t.cy
}
t.offset = util.Constrain(t.offset, t.cy-height+1, t.cy)
// Adjustment
if count-t.offset < height {
t.offset = util.Max(0, count-height)