Js - #11
Open
J494-bit wants to merge 686 commits into
Open
Conversation
Author
|
Go |
ghost
approved these changes
Jan 5, 2023
Speedytats
approved these changes
Mar 8, 2023
Speedytats
approved these changes
Mar 8, 2023
harshsingh32
approved these changes
Apr 18, 2023
cesarapascual
approved these changes
Apr 19, 2023
* Make sure not to break out-of-margin emtpy-sig
* preview_new_release: Remove alcotest and tezos * preview_new_release: Option to apply a previous version first An executable can be passed to the new `-b` option, it will be applied in a first pass and the result will be commited. This ensures that the preview commit shows only the diff caused by the new version.
These files slow down backporting upstream changes as well as making further changes to the extended parser.
Structure items with an extension were formatted in a compact way when `module-item-spacing=compact` even though the similar items without an extension would not. This is a problem in future work where the `Pstr_extension` node is no longer used.
Co-authored-by: Jules Aguillon <jules@j3s.fr>
A break is added after wrapping string constants in argument lists. This break was missing for string constants containing explicit line breaks or format hints.
This argument doesn't work similarly to the `~epi` argument of other functions. It meant different things depending on the expression: - `Pexp_match` and `Pexp_apply`: it was similar to `~pro`. - `Pexp_constant`: like the usual `~epi` but was passed a break. - all other cases: it was not used. These rules are now handled in `fmt_args_grouped`, the only place where they made sense. The `~epi` argument is removed. In the first case, the `~pro` argument is used instead. This requires refactoring `fmt_expression` to make sure that every cases use `~pro` exactly once.
This fixes a regression introduced in #1672 making --no-comment-check less useful.
#2463) Rather than formatting comment code blocks into a string, and then converting that string back into an Fmt.t, we instead just generate the Fmt.t directly, which allows the styling to know about current indentation without needing to manually indent/unindent the formatted text.
* Define type 'function_param' * Add Fp in the Ast context * Replace Sugar.arg_kind with function_param
* Missing parens in module type `(M with type ..) -> N` Fix the parenthesing rules for `with type` inside functors and functors inside `with type`. * Fix short-syntax functor formatting
* use special case beginend syntax in match cases when possible * also allow `if then begin match` * changelog * changelog * fmt
Co-authored-by: Jules Aguillon <jules@j3s.fr>
* parser-std: Backport polymorphic paramters From ocaml/ocaml#13806 * parser-ext: Backport polymorphic parameters * Fix Ast rules for Ptyp_poly It is treated with the same precedence as Ptyp_arrow.
* parser-std: Backport Ppat_unpack
Also backport the `map_loc` change in `Ast_mapper`.
* parser-ext: Backport Ppat_unpack
We already did a similar change in the past, the parsetree doesn't
change.
* parser-ext: Backport Ast_mapper map_loc changes
* Disable rewriting rule for Ppat_unpack
It is no longer valid to rewrite:
((module M) : (module S))
into:
(module M : S)
The original string syntax (eg. `{del| .. |del}`) is not respected
because this is probably unecessary.
An minimal unsafe version of Domain.DLS is used to implement Format_. This shim module just needs to be removed if OCamlformat ever use parallelism. * Disable scan_push bug fix This bug fix breaks OCamlformat and will be fixed in a separate PR.
Improve the formatting of:
module type of M
with module A = A (*test*)
`Pexp_struct_item` replaces `Pexp_letexception`, `Pexp_letmodule` and `Pexp_letopen`. The code for the old constructs is removed and replaced by a call to `fmt_structure_item`. The code for formatting structure items is changed to propagate the `~pro` and `~epi` arguments. * Extend the context for embedded struct items The `Str_exp` context is needed to apply the `let_module` option. The `Mb` context constructor is extended to carry the parent context, which would otherwise be erased before `fmt_module` gets called.
Backport modular explicits syntax. The test code is taken from the compiler's testsuite. * Factorize the formatting of package types This removes code with seamingly no regressions.
Backport refactors and other changes made upstream that do not change the language syntax. The goal is to reduce the diff with upstream to make future updates easier. * Reduce diff in arg_label The location is different but no comment moved in the tests.
* Add more tests for let-struct-items Taken from the compiler's testsuite.
This reduce the indentation of the `begin` keyword in this case:
let () =
if true then (* a *)
begin
()
end
else ()
and fixes an instability issue when the comment was big enough.
The special handling of empty files by the (diff) rule is removed in Dune 3.22: ocaml/dune#13696 Promote the empty error files to avoid build errors.
This was caught by opam-repository CI and caused by the vendored modules from the compiler's source.
…it-or-vertical` (#2797) * Fix instability with if-then-else=fit-or-vertical and comments after then When using if-then-else=fit-or-vertical, a comment placed between the `then` keyword and the branch expression caused an infinite oscillation between two formats: then (* comment *) <--> then expr (* comment *) expr The root cause is that the hovbox's soft break (break 1 2) in branch_pro allows the comment to fit on the same line as `then` when the hovbox doesn't break. On re-parsing, the comment is then classified as 'after the then keyword' instead of 'before the expression', producing a different layout that breaks back to the other form. Fix: in the Fit_or_vertical case, when the expression is multi-line and there is no begin..end or keyword comment, use cmts_before_opt to check for and consume comments before the branch body directly in branch_pro, placing them with a forced line break so the comment stays on its own indented line. The single-line check ensures short expressions that fit on one line are unaffected.
…vertical (#2798) When a `match expr with` appeared as a branch of `if-then-else` with `if-then-else=fit-or-vertical`, the match keyword, scrutinee, and `with` were broken onto separate lines even when they easily fit on one line: if true then match expr with | true -> () | false -> () The regression was introduced by 8056fa5 ("Overhaul begin match formatting"), which changed `fmt_match` to wrap `pro` inside a nested `hvbox 0 (pro_inner $ keyword)`. When the match is inside an if-then-else branch, the `pro` carries a `break_unless_newline 1000 2` (from the Fit_or_vertical branch_pro). This inflates the inner hvbox's size to 1000+, causing the Format engine to classify the parent hovbox as non-fitting, which forces all breaks in the `match .. expr .. with` box to break. Fix: in `match_inner_pro`, return `false` for `Pexp_ifthenelse` context (same as already done for `Pexp_infix`), so that `pro` is placed outside the match's inner hvbox as `pro_outer`, where it does not interfere with the match keyword/expr/with fitting on one line.
* Fix formatting oscillation in if-then-else branches When a comment sits between a keyword (then/else) and certain branch expressions (begin...end wrapping match/try/function/ifthenelse, or bare match/try/function/ifthenelse), the comment would alternate between "after keyword" and "before expression" placements across formatting iterations. This caused branch_pro to compute different indentation each time, producing "formatting did not stabilize after 10 iterations". The fix detects these oscillation-prone patterns and ensures stable formatting regardless of the initial comment placement: - In Fmt_ast, when a branch matches `needs_raw_cmts_after_kw`, extract "after keyword" comments without breaks and override branch_pro using `Params.raw_cmts_branch_pro` (which computes mode-appropriate break+indent+comment output). - In Params, `branch_pro_with_cmts` detects comments "before expression" (the alternate placement) and forces a break, producing the same layout as the "after keyword" path. - For begin...end branches, `branch_pro` checks for comments before the inner expression (not just the begin...end node) to handle the case where the comment migrates inside the begin...end scope. Affected modes: Fit_or_vertical, Compact (profile=conventional). Assisted-by: GitHub Copilot:claude-opus-4-20250514
The root cause was that the ocamlformat executable's internal module
name was main, so dune built `main.exe`. The program derives its
self-name (used in error messages) from `argv.(0)` at
`lib/Translation_unit.ml:37`. Under dune package management:
(setup-dune@v2), `%{bin:ocamlformat}` resolves to the raw `main.exe`
instead of the install-layout `ocamlformat` path, making the
program call itself `main` instead of `ocamlformat`.
Now `argv.(0)` ends in `ocamlformat.exe` in every build mode, so
`exe = "ocamlformat"` consistently. `dune build @test/failing/runtest`
passes locally, and the change is independent of how dune resolves the
binary, so it'll be stable on CI too.
The separate `bin/ocamlformat-rpc/` executable (also internally named
`main`) is untouched.
handle_last_newline scans backward and matches '\n' immediately, leaving
the preceding '\r' (if any) on the stripped string. Because deindent then
splits on '\n' and rejoins with '\n', every '\r' on an intermediate line
ends up followed by a '\n' — but the '\r' on the last line is bare,
since String.concat puts no separator after the final element.
The OCaml lexer's newline regex is '\r*\n', so a bare trailing '\r'
falls through to the Illegal_character rule. This caused the Windows CI
to fail on any {@ocaml[...]} block in a CRLF-checked-out file.
Mirror handle_first_newline (which already eats a leading '\r' before
matching '\n'): when handle_last_newline finds '\n', also drop a
preceding '\r' so the full CRLF is consumed.
* Params: deduplicate branch_pro_with_cmts across if-then-else modes The `Compact` and `Fit_or_vertical` arms of `get_if_then_else` each carried a near-identical block computing `branch_pro` for the case of a comment preceding a special branch body, and their inline break expressions (`break 1000 0 $ cmts $ break 1000 0` and `break 1000 2 $ cmts`) duplicated exactly what `raw_cmts_branch_pro` already computes per mode. Extract a single `branch_pro_with_cmts ~default ~guard` helper that reuses `raw_cmts_branch_pro`, parameterized by each mode's comment-less `branch_pro` and its extra applicability guard. Behavior is unchanged (the full test suite passes with no ref changes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix indentation of bare match branch after comment with fit-or-vertical With `if-then-else=fit-or-vertical`, an `if … else (* cmt *) match … with` branch (a bare match/function/try/if, not wrapped in begin/end or parentheses) rendered the branch body indented to the comment's end column instead of under the comment. `Params.raw_cmts_branch_pro` emitted, for non-Compact modes, `break 1000 2 $ cmts` with no trailing break. A bare branch's body breaks with `break_unless_newline`, a no-op unless already at the beginning of a line, and `fmt_match` opens its box at the current column — the comment's end column. Add an optional `~bare_branch` flag that appends a trailing break so the body box opens at the line start. It is not set for begin/end- or paren-wrapped branches, which emit their opening delimiter right after the comment, keeping that rendering unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix begin match/if branch layout with fit-or-vertical With `if-then-else=fit-or-vertical`, an `else begin match … with … end` branch rendered the header split over several lines (`begin match` / scrutinee / `with`), `begin if … end` likewise, and `end` ended up less indented than `begin`. For the `begin match/try/function/if end` shortcut, `fmt_beginend` threaded the branch `pro` into the inner expression's `pro`. In fit-or-vertical (and vertical) that `pro` is a `break_unless_newline 1000`, and because the inner expression's ctx0 is the begin/end node, `match_inner_pro` places it inside the header box. `pp_print_or_newline` enqueues its full width (1000) as the token length, so the header box never fits and is forced to break. When the begin/end is an if-then-else branch, emit `pro` outside the box (the branch break already positions it) and wrap the body and `end` together so `end` lines up with `begin`. Other contexts (e.g. `map x begin fun … end`) keep `pro` inside to preserve their indentation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Revert "Fix begin match/if branch layout with fit-or-vertical" This reverts commit e3babbb. * Rework begin match/if branch layout (review follow-up) Reworks the previous fix (e3babbb) per review. The earlier version special-cased `ctx0=ifthenelse` in `fmt_beginend` to handle a comment before a `begin match/if … end` branch body; it glued the body (`begin if a then b`) and reindented it when a comment was added. Why the comment case now goes through the plain begin/end path: `fmt_beginend` cannot see the comment — `branch_pro` consumes it in `Params` before the expression is formatted. `get_if_then_else`, however, runs before consumption, so the decision belongs there. A special `begin match/if … end` branch with a leading comment on its body no longer takes the `begin <kw>` shortcut arm; it falls through to the existing plain begin/end machinery (`has_cmts_before` predicate). That keeps `begin` on its own line with the body (comment included) one indent in, identically across all if-then-else modes, so adding a comment no longer reindents the body. Why expr_pro is dropped for special-bodied begin/end branches: the fit-or-vertical / vertical branch `expr_pro` is `break_unless_newline 1000`, which enqueues width 1000 even as a no-op at BOL and poisons the body's `match … with` / `if … then` header box, forcing it to split over several lines. Such a body already breaks after `begin`, so it provides its own break instead of the poisoning one. Simple-bodied `begin e end` keeps the regular branch break (no header to poison). K&R additionally dropped the extra body box that indented a comment-routed body by +2. Why a ctx0 box wrap remains in fmt_beginend: the no-comment shortcut path still needs `end` aligned with `begin`. In fit-or-vertical the branch box is `hovbox 0` while the branch break indents `begin` one level in, so the body and `end` are wrapped together. This is scoped to `ctx0=ifthenelse` because wrapping unconditionally reindents application arguments (`map x begin fun … end`, exp_grouping.ml); the scope is structural (alignment), not comment-handling logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With dune-pkg, the executable could be named "ocamlformat.exe" and "main.exe", creating a diff in the generated manpage.
* Fix comment-before-paren gluing in fit-or-vertical ITE branches
When a comment precedes a parenthesized match/try/function/if branch
(janestreet and ahrefs profiles with parens_ite or exp_grouping=parens),
glue the opening paren to the content instead of putting it alone on a
line. Before: `(* cmt *)\n(\nmatch e with\n| ...)`
After: `(* cmt *)\n(match e with\n | ...)`
Also simplify raw_cmts_branch_pro: remove the bare_branch distinction
(always emit trailing break), since paren-glue now handles the case that
bare_branch was partially covering.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: Add regression test for comment before [begin match ... end]
Tests the paren-glue behavior: with exp_grouping=parens (janestreet,
ahrefs), [begin match ... end] is parsed as a bare match needing
parenthesization; a preceding comment must not glue to the opening
paren.
Without the fix, this renders as:
(* a comment *)begin match some_long_scrutinee_expression with
| A -> a
| B -> b
end
or (with exp_grouping=parens):
(* a comment *)
(
match some_long_scrutinee_expression with
| A -> a
| B -> b)
Note: without the preceding fix, janestreet + k-r mode has a
pre-existing over-indentation issue for match-in-parens branches (match
indented +2 relative to the comment inside the parens). This caused
non-convergence with max-iters=3 in the unfixed code; the paren-glue
fix resolves the oscillation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Update CHANGES
---------
Co-authored-by: Boris Yakobowski <byakobow@mathworks.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Jules Aguillon <jules@j3s.fr>
Concretely, this is what changes:
```diff
while
loooong
condition
-do
- ()
-done
+do () done
```
A real example where I wished for a more compact formatting:
ocaml/ocaml-re@59198dd#diff-a91a869b078e8995843c8b377da588505468c962166d62d7029893c27be32833R70-R80
To explain a bit: while-loops are fairly inexpressive in ocaml due to the absence of
`break`. In a decent number of cases, it's nicer to give up on the separation between
condition and body, by moving the body into the condition. In that case, it's a bit
silly to waste 3 lines on the empty body.
For instance, if you wanted to write a loop where the first iteration always runs, like
a do-while in C, the obvious thing to do (without auxiliary functions) is:
```ocaml
let first = ref true in
while !first || condition; do first := false; body done
```
but the simpler version is:
```ocaml
while body; condition do () done
```
Or iterating through a stack:
```ocaml
while not (Stack.is_empty s) do f (Stack.pop_exn s) done
```
can be done without double-checking that the stack is empty:
```ocaml
while
match Stack.pop s with
| None -> false
| Some elt -> f elt; true
do () done
```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Me gustaría probar