feature: Suggest candidates when alias not found by Alizter · Pull Request #7004 · ocaml/dune

Alizter · 2023-02-05T18:32:15Z

We now suggest candidates when an alias is not found. Error messages look like:

$ dune build @fou
Error: Alias "fou" specified on the command line is empty.
It is not defined in . or any of its descendants.
Hint: did you mean fmt or foo?
[1]

changelog
tests

bin/alias.ml

Alizter · 2023-02-09T15:25:56Z

@emillon Would you like this in 3.7?

emillon · 2023-02-09T19:17:24Z

That's "just" a CLI change that's low risk but still a UX improvement so yes I don't mind including it.

bin/alias.ml

rgrinberg · 2023-02-11T19:58:10Z

bin/alias.ml

+            ~f:(fun _ a _ -> Some a)
+            build.aliases (aliases t)
+        | _ :: t -> aliases t
+        | _ -> Alias.Name.Map.empty


I don't think this is right. You should use --context or default to make the suggestion.

As discussed in chat, getting hold of context_arg from the CLI is a bit tricky here as I would have to thread it through unrelated places.

bin/alias.ml

Alizter · 2023-02-12T17:40:35Z

So there is an issue with this PR as pointed out by Rudi. The suggested aliases will only appear from the first context considered when generating the hints. However for most users, I feel as though this will not matter. The correct fix IMO, would be to get hold of Common.context_arg globally and pick the currently selected context for generating the hints. I don't think I will do that in this PR however.

emillon · 2023-02-13T10:41:33Z

test/blackbox-tests/test-cases/alias-candidates.t/run.t

@@ -0,0 +1,8 @@
+Dune should suggest similar aliases when it cannot find one. 


Can you turn that into a file cram test ? (alias-candidate.t with cat > dune and cat > dune-project in it)

Sure, but why?

that's fewer files, tests are more self-contained. I think we said we now prefer that for new tests.

emillon · 2023-02-13T10:43:55Z

This is an improvement even if it's not correct in the presence of several contexts. Fine to include it but please write a ticket or a test to document the followup. Or postpone to 3.8 as you prefer.

Alizter · 2023-02-13T14:24:46Z

@emillon I'll write a ticket for the followup.

#7060

rgrinberg · 2023-02-13T16:03:07Z

The bigger problem with this feature is that it doesn't list the aliases defined in subdirectories. We might want to leave a note on that.

Alizter · 2023-02-14T02:03:07Z

OK I've decided against including this in 3.7. Let me implement this properly in 3.8.

emillon · 2023-04-19T14:20:14Z

@Alizter what do you want to do about this one for 3.8?

Alizter · 2023-07-12T14:09:20Z

So updating my position on this PR. @rgrinberg asked to limit the alias query to the build context however this doesn't make sense to me for a few reasons.

dune build doesn't take --context the closest is -x.
Aliases passed to dune build build in all contexts.

Therefore I think the status of the PR is good and ready to go.

ocaml-benchmarks · 2023-07-12T20:35:07Z

#7004 (5338d67) changes the metrics as follows in comparison to main (4256431) when running on fermat (bench/monorepo/bench.Dockerfile):

Benchmark: default

Test: dune monorepo benchmarks

build from scratch changed by 2.0%
null build changed by -5.2%
watch mode: changing file in 'base' library changed by 1.3%
watch mode: changing file in 'file_path' library changed by 0.8%
watch mode: fixing error in file in 'base' library changed by -2.4%
watch mode: fixing error in file in 'file_path' library changed by 0.3%
watch mode: introducing error in file in 'base' library changed by -2.3%
watch mode: introducing error in file in 'file_path' library changed by -8.2%

ocaml-benchmarks · 2023-07-13T01:50:49Z

#7004 (7864e0d) changes the metrics as follows in comparison to main (a0145b2) when running on fermat (bench/monorepo/bench.Dockerfile):

Benchmark: default

Test: dune monorepo benchmarks

build from scratch changed by 5.0%
null build changed by 3.9%
watch mode: changing file in 'base' library changed by -0.4%
watch mode: changing file in 'file_path' library changed by -5.4%
watch mode: fixing error in file in 'base' library changed by -2.8%
watch mode: fixing error in file in 'file_path' library changed by -3.2%
watch mode: introducing error in file in 'base' library changed by 1.2%
watch mode: introducing error in file in 'file_path' library changed by -12.3%

bin/alias.ml

rgrinberg

Not perfect, but good enough for now. Some stylistic issues.

bin/alias.ml

Alizter · 2023-10-20T14:37:06Z

@rgrinberg I've made the stylistic changes you suggested and updated the changelog to the new directory.

rgrinberg · 2023-10-20T00:03:13Z

CHANGES.md

 - Bump minimum version of the dune language for the melange syntax extension
  from 3.7 to 3.8 (#7665, @jchavarri)

+- Dune now suggests other aliases when an alias is not found (#7004, @Alizter)


this doesn't need a CHANGES entry

I think this comment is out of date since I moved the changelog to its own file before you reviewed. Are you also suggesting I remove that?

I've removed that too for now, if we need a changelog we can just write a new one at a later date.

We usually don't write CHANGES entry for error message improvements

bin/alias.ml

Signed-off-by: Ali Caglayan <alizter@gmail.com>

Alizter · 2023-10-23T12:44:09Z

@rgrinberg I've removed the changelog entry and refactored the accumulation of name suggestions to use a much shorter Alias.Name.Set.union_map.

rgrinberg reviewed Feb 5, 2023

View reviewed changes

bin/alias.ml Outdated Show resolved Hide resolved

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from 9e0d15e to 1e9854a Compare February 5, 2023 18:55

Alizter marked this pull request as ready for review February 5, 2023 18:56

Alizter commented Feb 5, 2023

View reviewed changes

bin/alias.ml Outdated Show resolved Hide resolved

Alizter requested a review from rgrinberg February 5, 2023 18:57

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch 3 times, most recently from 24707b7 to b295392 Compare February 9, 2023 15:20

emillon reviewed Feb 9, 2023

View reviewed changes

bin/alias.ml Outdated Show resolved Hide resolved

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from b295392 to 3029fe6 Compare February 9, 2023 19:29

rgrinberg reviewed Feb 11, 2023

View reviewed changes

bin/alias.ml Outdated Show resolved Hide resolved

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from 3029fe6 to d02575a Compare February 12, 2023 17:33

Alizter added this to the 3.7.0 milestone Feb 12, 2023

emillon reviewed Feb 13, 2023

View reviewed changes

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from d02575a to adcd3a5 Compare February 13, 2023 14:05

Alizter mentioned this pull request Feb 13, 2023

Alias hint generation should only use selected build context #7060

Open

Alizter modified the milestones: 3.7.0, 3.8.0 Feb 14, 2023

Alizter modified the milestones: 3.8.0, 3.9.0 Apr 19, 2023

Alizter mentioned this pull request May 22, 2023

feature: dune show targets and dune show aliases #7770

Merged

3 tasks

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from adcd3a5 to 5338d67 Compare July 12, 2023 14:06

Alizter requested review from emillon and rgrinberg July 14, 2023 12:01

rgrinberg reviewed Oct 20, 2023

View reviewed changes

bin/alias.ml Outdated Show resolved Hide resolved

rgrinberg requested changes Oct 20, 2023

View reviewed changes

bin/alias.ml Outdated Show resolved Hide resolved

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch 5 times, most recently from c44ec51 to fad6141 Compare October 20, 2023 14:35

rgrinberg reviewed Oct 23, 2023

View reviewed changes

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from fad6141 to ed57ce5 Compare October 23, 2023 12:39

feature: Suggest candidates when alias not found

12c9365

Signed-off-by: Ali Caglayan <alizter@gmail.com>

Alizter force-pushed the ps/rr/feature__suggest_candidates_when_alias_not_found branch from ed57ce5 to 12c9365 Compare October 23, 2023 12:41

rgrinberg merged commit af9e838 into ocaml:main Oct 24, 2023

Alizter deleted the ps/rr/feature__suggest_candidates_when_alias_not_found branch October 24, 2023 09:52

		@@ -0,0 +1,8 @@
		Dune should suggest similar aliases when it cannot find one.

Conversation

Alizter commented Feb 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alizter commented Feb 9, 2023

Uh oh!

emillon commented Feb 9, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Alizter commented Feb 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emillon commented Feb 13, 2023

Uh oh!

Alizter commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Feb 13, 2023

Uh oh!

Alizter commented Feb 14, 2023

Uh oh!

emillon commented Apr 19, 2023

Uh oh!

Alizter commented Jul 12, 2023

Uh oh!

ocaml-benchmarks bot commented Jul 12, 2023

Benchmark: default

Test: dune monorepo benchmarks

Uh oh!

ocaml-benchmarks bot commented Jul 13, 2023

Benchmark: default

Test: dune monorepo benchmarks

Uh oh!

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Alizter commented Oct 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Alizter commented Oct 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Alizter commented Feb 5, 2023 •

edited

Loading

Alizter commented Feb 13, 2023 •

edited

Loading