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.

Sign up for GitHub

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

ReScript cli clean up + watcher fix #6404

Merged
merged 4 commits into from
Sep 27, 2023
Merged

ReScript cli clean up + watcher fix #6404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56b7649
Watch mode clean up
DZakh Sep 14, 2023
4d1f7fb
Update clean command help
DZakh Sep 20, 2023
56c464b
Update changelog
DZakh Sep 20, 2023
70b514a
Update CHANGELOG.md
DZakh Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

#### :bug: Bug Fix

- Fix issue with GenType and labelled arguments https://github.com/rescript-lang/rescript-compiler/pull/6406
- Fix issue with GenType and labelled arguments. https://github.com/rescript-lang/rescript-compiler/pull/6406
- Fix dependencies reinitialization on every change in watch mode. Leads to faster rebuilds and cleaner terminal. https://github.com/rescript-lang/rescript-compiler/pull/6404

#### :rocket: New Feature

Expand All @@ -29,6 +30,9 @@
- The error message for "toplevel expressions should evaluate to unit" has been revamped and improved. https://github.com/rescript-lang/rescript-compiler/pull/6407
- Improve "Somewhere wanted" error messages by changing wording and adding more context + suggested solutions to the error messages where appropriate. https://github.com/rescript-lang/rescript-compiler/pull/6410
- Add smart printer for pipe-chains. https://github.com/rescript-lang/rescript-compiler/pull/6411
- Display the compile time for `rescript build` command. https://github.com/rescript-lang/rescript-compiler/pull/6404
- Improve help message for `build` and `clean` commands. https://github.com/rescript-lang/rescript-compiler/pull/6404
- Pass through the `-verbose` flag to builds in watch mode. https://github.com/rescript-lang/rescript-compiler/pull/6404

# 11.0.0-rc.3

Expand Down
30 changes: 15 additions & 15 deletions jscomp/bsb_exe/rescript_main.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ let () = Bsb_log.setup ()

let separator = "--"

let watch_mode = ref false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've separated the watch mode from other delegated commands to handle it separately. So now we have better control over it, and the ref is unnecessary.

Before:

  • Trigger build and exit with 1
  • Start watching changes
  • Trigger build

After:

  • Trigger build
  • Start watching changes

let no_deps_mode = ref false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first build should be done with dependencies. Builds on change should be without dependencies.


let do_install = ref false

Expand Down Expand Up @@ -84,13 +84,13 @@ let ninja_command_exit (type t) (ninja_args : string array) : t =
ninja -C _build
*)
let clean_usage =
"Usage: rescript.exe clean <options>\n\n\
`rescript clean` only cleans the current project\n"
"Usage: rescript clean <options>\n\n\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed exe and updated the description. It's more relevant now, since the -with-deps is deprecated.

`rescript clean` cleans build artifacts\n"

let build_usage =
"Usage: rescript.exe build <options> -- <ninja_options>\n\n\
`rescript build` implicitly builds dependencies if they aren't built\n\n\
`rescript.exe -- -h` for Ninja options (internal usage only; unstable)\n"
"Usage: rescript build <options> -- <ninja_options>\n\n\
`rescript build` builds the project with dependencies\n\n\
`rescript -- -h` for Ninja options (internal usage only; unstable)\n"

let install_target () =
let ( // ) = Filename.concat in
Expand All @@ -114,23 +114,24 @@ let build_subcommand ~start argv argv_len =
?finish:(if i < 0 then None else Some i)
~argv
[|
("-w", unit_set_spec watch_mode, "Watch mode");
("-w", unit_set_spec (ref false), "Watch mode");
( "-ws",
string_set_spec (ref ""),
"[host]:port set up host & port for WebSocket build notifications" );
("-verbose", call_spec Bsb_log.verbose, "Set the output to be verbose");
Comment on lines +117 to +121
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorted in the order:

  • Public
  • Deprecated
  • Internal

("-with-deps", unit_set_spec (ref true), "*deprecated* This is the default behavior now. This option will be removed in a future release");
( "-install",
unit_set_spec do_install,
"*internal* Install public interface files for dependencies" );
(* This should be put in a subcommand
previously it works with the implication `bsb && bsb -install`
*)
( "-ws",
string_set_spec (ref ""),
"[host]:port set up host & port for WebSocket build notifications" );
( "-regen",
unit_set_spec force_regenerate,
"*internal* \n\
Always regenerate build.ninja no matter bsconfig.json is changed or \
not" );
("-verbose", call_spec Bsb_log.verbose, "Set the output to be verbose");
("-no-deps", unit_set_spec no_deps_mode, "*internal* Needed for watcher to build without dependencies on file change");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a usecase for it, so I've added the flag as internal

|]
failed_annon;

Expand All @@ -143,18 +144,17 @@ let build_subcommand ~start argv argv_len =
let config_opt =
Bsb_ninja_regen.regenerate_ninja ~package_kind:Toplevel
~per_proj_dir:Bsb_global_paths.cwd ~forced:!force_regenerate in
Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt ninja_args;
if not !no_deps_mode then Bsb_world.make_world_deps Bsb_global_paths.cwd config_opt ninja_args;
if !do_install then install_target ();
if !watch_mode then exit 0 (* let the watcher do the build*)
else ninja_command_exit ninja_args
ninja_command_exit ninja_args

let clean_subcommand ~start argv =
Bsb_arg.parse_exn ~usage:clean_usage ~start ~argv
[|
("-verbose", call_spec Bsb_log.verbose, "Set the output to be verbose");
( "-with-deps",
unit_set_spec (ref true),
"*deprecated* This is the default behavior now. This option will be removed in a future release" );
("-verbose", call_spec Bsb_log.verbose, "Set the output to be verbose");
|]
failed_annon;
Bsb_clean.clean_bs_deps Bsb_global_paths.cwd;
Expand Down
Loading