Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make it possible to invoke magic-trace run -- prog args #292

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

v-gb
Copy link
Contributor

@v-gb v-gb commented Feb 9, 2024

instead of magic-trace run prog -- args.

The current CLI is annoying because you can't just prefix the command line with magic-trace run, you have to insert of -- in between the prog and the args.

I didn't actually run the result, because I get a number of type errors. I fixed enough to know my code types but not everything. Example of what I had to fix:

diff --git a/src/for_range.ml b/src/for_range.ml
index 5ddf0b0fff...4181efe477 100644
--- a/src/for_range.ml
+++ b/src/for_range.ml
@@ -21,7 +21,7 @@
 let range_hit_times ~decode_events ~range_symbols =
   let open Deferred.Or_error.Let_syntax in
   let%bind { Decode_result.events; _ } = decode_events () in
-  Deferred.List.map events ~f:(fun events ->
+  Deferred.List.map events ~how:`Sequential ~f:(fun events ->
     let { Trace_filter.start_symbol; stop_symbol } = range_symbols in
     let is_start symbol = String.(Symbol.display_name symbol = start_symbol) in
     let is_stop symbol = String.(Symbol.display_name symbol = stop_symbol) in

instead of `magic-trace run prog -- args`.

The current CLI is annoying because you can't just prefix the command
line with `magic-trace run`, you have to insert of `--` in between
the prog and the args.

Signed-off-by: Valentin Gatien-Baron <valentin.gatienbaron@gmail.com>
Copy link
Member

@Xyene Xyene left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks! (And hi!)

We're still on core 0.15 which is what's causing your compilation errors. If you have a switch for ocaml 4.13 I think that forces the constraints to resolve to core 0.15, which will get this repo building.

We should upgrade though, I'll take a look at doing that.

@Xyene Xyene merged commit 9846bd7 into janestreet:master Feb 17, 2024
2 checks passed
@Xyene
Copy link
Member

Xyene commented Feb 17, 2024

#294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants