Improve error messages on un-resolvable fuzzy command invocations #4573

ChrisPenner · 2024-01-04T00:31:22Z

Overview

For fuzzy finds with no options

Before

.empty> view
.empty>

After

.empty> view

⚠️

Sorry, I was expecting an argument for the definition to view, and I couldn't find any to suggest to you. 😅
.empty>

For fuzzy finds with no resolvers and a missing arg

Before:

.> move.term

<fzf select a term to rename, then since there's no resolver for the second arg, the command just fails and goes back to the prompt>

.>

After:

.> move.term
`move.term foo bar` renames `foo` to `bar`.

Without this patch some command invocations can behave unintuitively, e.g. running move.term without any args will fuzzy search for the first arg, but then will silently not run anything because there's no fzf resolver for the new name argument which is required.

This changes it so we validate that we have resolvers for all required arguments before we actually invoke any of them.

It also will fail with a nice message if a fuzzy-finder is invoked but doesn't have any options to select from.

Implementation notes

Collect all the required fzf resolvers in one loop, then actually run them in a second loop so that we don't start fuzzy-finding unless we know we have resolvers for every argument we need them for.
Change the error handling for resolvers so we know why fuzzy resolve failed and print an error accordingly.
Change the transcript parser to accept argument parse errors as valid errors w/r to ucm:error
Adds argument names to every arg in InputPatterns so we can tell the user which arg they're selecting. These arg names may also be handy in the future for generating help messages or something like it.

Test coverage

Added a couple transcript cases

ChrisPenner · 2024-01-04T19:02:49Z

unison-src/transcripts/fuzzy-options.output.md

@@ -33,7 +53,7 @@ Definition args

 .> debug.fuzzy-options view _

-  Select a definition:


Unfortunately generic articles are complicated in English (a vs an), but I think things read okay without it.

I think it'll be good to improve this at some point to be consistent with the minimally-robotic UCM message style, but not going to delay the PR.

ChrisPenner · 2024-01-04T19:03:33Z

unison-cli/src/Unison/CommandLine/FZFResolvers.hs

@@ -9,6 +9,15 @@ module Unison.CommandLine.FZFResolvers
    projectBranchOptionsWithinCurrentProject,
    fuzzySelectFromList,
    multiResolver,
+    definitionResolver,


Now that we don't need custom arg descriptions in each resolver, we can just export the whole resolver type :)

ChrisPenner · 2024-01-04T19:03:55Z

parser-typechecker/src/Unison/PrettyPrintEnv/Names.hs

ChrisPenner · 2024-01-04T19:04:00Z

parser-typechecker/src/Unison/Syntax/FileParser.hs

ChrisPenner · 2024-01-04T19:04:55Z

unison-cli/src/Unison/Codebase/TranscriptParser.hs

@@ -358,7 +358,14 @@ run verbosity dir stanzas codebase runtime sbRuntime nRuntime config ucmVersion
                        let getRoot = fmap Branch.head . atomically $ readTMVar rootVar
                        liftIO (parseInput codebase getRoot curPath numberedArgs patternMap args) >>= \case
                          -- invalid command is treated as a failure
-                          Left msg -> liftIO (dieWithMsg $ Pretty.toPlain terminalWidth msg)
+                          Left msg -> do


Previously a failure in argument parsing would crash the transcript even if inside a ucm:error block, now it prints the error the output but proceeds if it was an expected error.

ChrisPenner · 2024-01-04T19:06:20Z

unison-cli/src/Unison/CommandLine/FZFResolvers.hs

@@ -38,48 +47,44 @@ import Unison.Util.Relation qualified as Relation
 type OptionFetcher = Codebase IO Symbol Ann -> ProjectContext -> Branch0 IO -> IO [Text]

 data FZFResolver = FZFResolver
-  { argDescription :: Text,


No longer need a description in the resolver since we have one on each arg.

ChrisPenner added 2 commits January 3, 2024 16:28

Improve error messages on un-resolvable fuzzy command invocations

a093d5b

Update transcript

b7df4c4

ChrisPenner marked this pull request as ready for review January 4, 2024 00:42

Merge branch 'cp/fzf-args' into cp/fzf-better-errs

2219bb9

Base automatically changed from cp/fzf-args to trunk January 4, 2024 01:14

ChrisPenner added 3 commits January 4, 2024 10:25

Add arg names for everything

2709ea3

Merge branch 'cp/arg-names' into cp/fzf-better-errs-2

fd58bf5

Use arg names in fuzzy selections

5ab4b69

ChrisPenner commented Jan 4, 2024

View reviewed changes

parser-typechecker/src/Unison/PrettyPrintEnv/Names.hs Outdated

Copy link

Contributor Author

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ormolu

ChrisPenner commented Jan 4, 2024

View reviewed changes

parser-typechecker/src/Unison/Syntax/FileParser.hs Outdated

Copy link

Contributor Author

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ormolu

ChrisPenner commented Jan 4, 2024

View reviewed changes

ChrisPenner added 2 commits January 4, 2024 11:39

Update fzf failure errs

ae6abae

Update transcripts

acf4598

ChrisPenner requested a review from aryairani January 4, 2024 19:41

aryairani approved these changes Jan 4, 2024

View reviewed changes

aryairani added the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jan 4, 2024

ChrisPenner added 2 commits January 4, 2024 13:23

One last transcript re-run

ce05274

Use correct articles in Selection message

603859f

mergify bot merged commit 014b3fc into trunk Jan 4, 2024
7 checks passed

mergify bot deleted the cp/fzf-better-errs branch January 4, 2024 21:58

mergify bot removed the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Jan 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve error messages on un-resolvable fuzzy command invocations #4573

Improve error messages on un-resolvable fuzzy command invocations #4573

ChrisPenner commented Jan 4, 2024 •

edited

ChrisPenner Jan 4, 2024

aryairani Jan 4, 2024

ChrisPenner Jan 4, 2024

ChrisPenner Jan 4, 2024

ChrisPenner Jan 4, 2024

ChrisPenner Jan 4, 2024 •

edited

ChrisPenner Jan 4, 2024

		@@ -33,7 +53,7 @@ Definition args

		.> debug.fuzzy-options view _

		Select a definition:

Improve error messages on un-resolvable fuzzy command invocations #4573

Improve error messages on un-resolvable fuzzy command invocations #4573

Conversation

ChrisPenner commented Jan 4, 2024 • edited

Overview

For fuzzy finds with no options

For fuzzy finds with no resolvers and a missing arg

Implementation notes

Test coverage

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

aryairani Jan 4, 2024

Choose a reason for hiding this comment

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

ChrisPenner Jan 4, 2024 • edited

Choose a reason for hiding this comment

ChrisPenner Jan 4, 2024

Choose a reason for hiding this comment

ChrisPenner commented Jan 4, 2024 •

edited

ChrisPenner Jan 4, 2024 •

edited