From 89375005b5d3c68206362d9d1baacf9af18a981e Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Sat, 25 May 2024 16:20:37 +0900 Subject: [PATCH] Fix option validation order --- src/options.go | 91 ++++++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 39 deletions(-) diff --git a/src/options.go b/src/options.go index 202bfda9..0cfab20b 100644 --- a/src/options.go +++ b/src/options.go @@ -2616,6 +2616,51 @@ func validateSign(sign string, signOptName string) error { return nil } +func validateOptions(opts *Options) error { + if opts.Pointer != nil { + if err := validateSign(*opts.Pointer, "pointer"); err != nil { + return err + } + } + + if opts.Marker != nil { + if err := validateSign(*opts.Marker, "marker"); err != nil { + return err + } + } + + if !tui.IsLightRendererSupported() && opts.Height.size > 0 { + return errors.New("--height option is currently not supported on this platform") + } + + if opts.Scrollbar != nil { + runes := []rune(*opts.Scrollbar) + if len(runes) > 2 { + return errors.New("--scrollbar should be given one or two characters") + } + for _, r := range runes { + if uniseg.StringWidth(string(r)) != 1 { + return errors.New("scrollbar display width should be 1") + } + } + } + + if opts.Height.auto { + for _, s := range []sizeSpec{opts.Margin[0], opts.Margin[2]} { + if s.percent { + return errors.New("adaptive height is not compatible with top/bottom percent margin") + } + } + for _, s := range []sizeSpec{opts.Padding[0], opts.Padding[2]} { + if s.percent { + return errors.New("adaptive height is not compatible with top/bottom percent padding") + } + } + } + + return nil +} + // This function can have side-effects and alter some global states. // So we run it on fzf.Run and not on ParseOptions. func postProcessOptions(opts *Options) error { @@ -2644,30 +2689,6 @@ func postProcessOptions(opts *Options) error { opts.Marker = &defaultMarker } - if err := validateSign(*opts.Pointer, "pointer"); err != nil { - return err - } - - if err := validateSign(*opts.Marker, "marker"); err != nil { - return err - } - - if !tui.IsLightRendererSupported() && opts.Height.size > 0 { - return errors.New("--height option is currently not supported on this platform") - } - - if opts.Scrollbar != nil { - runes := []rune(*opts.Scrollbar) - if len(runes) > 2 { - return errors.New("--scrollbar should be given one or two characters") - } - for _, r := range runes { - if uniseg.StringWidth(string(r)) != 1 { - return errors.New("scrollbar display width should be 1") - } - } - } - // Default actions for CTRL-N / CTRL-P when --history is set if opts.History != nil { if _, prs := opts.Keymap[tui.CtrlP.AsEvent()]; !prs { @@ -2714,19 +2735,6 @@ func postProcessOptions(opts *Options) error { opts.Keymap[tui.DoubleClick.AsEvent()] = opts.Keymap[tui.CtrlM.AsEvent()] } - if opts.Height.auto { - for _, s := range []sizeSpec{opts.Margin[0], opts.Margin[2]} { - if s.percent { - return errors.New("adaptive height is not compatible with top/bottom percent margin") - } - } - for _, s := range []sizeSpec{opts.Padding[0], opts.Padding[2]} { - if s.percent { - return errors.New("adaptive height is not compatible with top/bottom percent padding") - } - } - } - // If we're not using extended search mode, --nth option becomes irrelevant // if it contains the whole range if !opts.Extended || len(opts.Nth) == 1 { @@ -2755,6 +2763,10 @@ func postProcessOptions(opts *Options) error { theme.Spinner = boldify(theme.Spinner) } + if err := opts.initProfiling(); err != nil { + return errors.New("failed to start pprof profiles: " + err.Error()) + } + return processScheme(opts) } @@ -2798,8 +2810,9 @@ func ParseOptions(useDefaults bool, args []string) (*Options, error) { return nil, err } - if err := opts.initProfiling(); err != nil { - return nil, errors.New("failed to start pprof profiles: " + err.Error()) + // 4. Final validation of merged options + if err := validateOptions(opts); err != nil { + return nil, err } return opts, nil