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

Fix parts parsing #155

Merged
merged 6 commits into from
Sep 6, 2019
Merged

Fix parts parsing #155

merged 6 commits into from
Sep 6, 2019

Conversation

Julow
Copy link
Collaborator

@Julow Julow commented Aug 6, 2019

Fixes #148.

If a part delimiter occur inside a nested structure or signature, it will be ignore:

[@@@part "1"]

module A =
struct

[@@@part "2"]

  type t = A

[@@@part "3"]

end

The parts 2 and 3 won't be parsed and everything will be the body of the part 1, which will be replaced by just:

[@@@part "1"]

module A =
struct

This PR fixes this by parsing the part delimiters line by line using Re instead of using OCaml's parser. This has the advantage of not failing and not depending on an unstable library.

@Julow Julow mentioned this pull request Aug 8, 2019
@samoht
Copy link
Collaborator

samoht commented Aug 19, 2019

I've restarted the failing CI job.

test/sync_to_md.md.expected Outdated Show resolved Hide resolved
@samoht
Copy link
Collaborator

samoht commented Aug 19, 2019

I think I'm ok with the approach -- it allows to put part in invalid ast fragments too, which is a nice feature (and allow to extract parts of structures as you demonstrated it).

I am not sure to understand where do the extra newlines come from.

Also, what happens if [@@@part ...] doesn't start at the beginning of the line? I got the impression that your patch is able to detect the part but then it removes these spaces in the expected file? I guess this will break ocamlformat-ed files, if you have a part deep into a structure.

@Julow
Copy link
Collaborator Author

Julow commented Aug 19, 2019

The regex that matches [@@@part ...] allow trailing whitespaces but they are not printed back. I didn't change the printing code so this is unchanged, except in the case the file did not parse and the whole file is printed as-is, including part separators.

@Julow
Copy link
Collaborator Author

Julow commented Aug 19, 2019

I added a commit (a21f0fa) to preserve the indentation of [@@@part ...].
Can be reverted. What do you think?

test/sync_to_ml.ml.expected Outdated Show resolved Hide resolved
@Julow Julow force-pushed the fix-parts-parsing branch from 95deb88 to a4470a6 Compare September 6, 2019 11:53
@Julow
Copy link
Collaborator Author

Julow commented Sep 6, 2019

Rebased. @samoht Do you think we can merge this PR ?

@samoht
Copy link
Collaborator

samoht commented Sep 6, 2019

LGTM

@samoht samoht merged commit e0a4def into realworldocaml:master Sep 6, 2019
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Oct 1, 2019
CHANGES:

#### Added

- Add `--syntax` option to `rule` subcommand to allow generating rules for cram
  tests (realworldocaml/mdx#177, @craigfe)
- Add a `require-package` label to explicitly declare dune `package` dependencies of a code block
  (realworldocaml/mdx#149, @Julow)
- Add an `unset-` label to unset env variables in shell blocks (realworldocaml/mdx#132, @clecat)

#### Changed

- Run promotion of markdown files before `.ml` files in generated dune rules (realworldocaml/mdx#140, @clecat)

#### Fixed

- Remove trailing whitespaces at the end of toplevel or bash evaluation lines
  (realworldocaml/mdx#166, @clecat)
- Improve error reporting of ocaml-mdx test (realworldocaml/mdx#172, @Julow)
- Rule: Pass the --section option to `test` (realworldocaml/mdx#176, @Julow)
- Remove trailing whitespaces from shell outputs and toplevel evals (realworldocaml/mdx#166, @clecat)
- Remove inappropriate empty lines in generated dune rules (realworldocaml/mdx#163, @Julow)
- Fix ignored `skip` label in `ocaml-mdx pp` (realworldocaml/mdx#1561, @craigfe)
- Fix synchronization of new parts from markdown to `.ml` (realworldocaml/mdx#156, @Julow)
- Fix ignored `[@@@Parts ...]` markers within module definitions (realworldocaml/mdx#155, @Julow)
- Fix a bug in internal OCaml version comparison that lead to crashes in some cases (realworldocaml/mdx#145, @gpetiot)
- Promote to empty `.ml` file when using `to-ml` direction (realworldocaml/mdx#139, @clecat)
- Apply `--force-output` to `.ml` file as well (realworldocaml/mdx#137, @clecat)
- Fix a bug preventing `.corrected` files to be written in some cases (realworldocaml/mdx#136, @clecat)
- Add compatibility with `4.09.0` (realworldocaml/mdx#133, @xclerc)

#### Removed

- Remove the `infer-timestamp` direction (realworldocaml/mdx#171 @Julow)
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Oct 3, 2019
CHANGES:

#### Added

- Add `--syntax` option to `rule` subcommand to allow generating rules for cram
  tests (realworldocaml/mdx#177, @craigfe)
- Add a `require-package` label to explicitly declare dune `package` dependencies of a code block
  (realworldocaml/mdx#149, @Julow)
- Add an `unset-` label to unset env variables in shell blocks (realworldocaml/mdx#132, @clecat)

#### Changed

- Format rules generated by `ocaml-mdx rule` using `dune format-dune-file` (realworldocaml/mdx#184, @NathanReb)
- Run promotion of markdown files before `.ml` files in generated dune rules (realworldocaml/mdx#140, @clecat)

#### Fixed

- Use module_presence information on Env.summary to prevent fetching absent modules from the
  toplevel env (realworldocaml/mdx#186, @clecat)
- Remove trailing whitespaces at the end of toplevel or bash evaluation lines
  (realworldocaml/mdx#166, @clecat)
- Improve error reporting of ocaml-mdx test (realworldocaml/mdx#172, @Julow)
- Rule: Pass the --section option to `test` (realworldocaml/mdx#176, @Julow)
- Remove trailing whitespaces from shell outputs and toplevel evals (realworldocaml/mdx#166, @clecat)
- Remove inappropriate empty lines in generated dune rules (realworldocaml/mdx#163, @Julow)
- Fix ignored `skip` label in `ocaml-mdx pp` (realworldocaml/mdx#1561, @craigfe)
- Fix synchronization of new parts from markdown to `.ml` (realworldocaml/mdx#156, @Julow)
- Fix ignored `[@@@Parts ...]` markers within module definitions (realworldocaml/mdx#155, @Julow)
- Fix a bug in internal OCaml version comparison that lead to crashes in some cases (realworldocaml/mdx#145, @gpetiot)
- Promote to empty `.ml` file when using `to-ml` direction (realworldocaml/mdx#139, @clecat)
- Apply `--force-output` to `.ml` file as well (realworldocaml/mdx#137, @clecat)
- Fix a bug preventing `.corrected` files to be written in some cases (realworldocaml/mdx#136, @clecat)
- Add compatibility with `4.09.0` (realworldocaml/mdx#133, @xclerc)

#### Removed

- Remove the `infer-timestamp` direction (realworldocaml/mdx#171 @Julow)
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Nov 29, 2019
CHANGES:

#### Added

- Add a `--output`/`-o` option to the `test` subcommand to allow specifying a different
  output file to write the corrected to, or to write it to the standard output (realworldocaml/mdx#194, @NathanReb)
- Migrate to OCaml 4.08 AST to add support for `let*` bindings (realworldocaml/mdx#190, @gpetiot)
- Add `--syntax` option to `rule` subcommand to allow generating rules for cram
  tests (realworldocaml/mdx#177, @craigfe)
- Add a `require-package` label to explicitly declare dune `package` dependencies of a code block
  (realworldocaml/mdx#149, @Julow)
- Add an `unset-` label to unset env variables in shell blocks (realworldocaml/mdx#132, @clecat)

#### Changed

- Format rules generated by `ocaml-mdx rule` using `dune format-dune-file` (realworldocaml/mdx#184, @NathanReb)
- Run promotion of markdown files before `.ml` files in generated dune rules (realworldocaml/mdx#140, @clecat)

#### Fixed

- Use module_presence information on Env.summary to prevent fetching absent modules from the
  toplevel env (realworldocaml/mdx#186, @clecat)
- Remove trailing whitespaces at the end of toplevel or bash evaluation lines
  (realworldocaml/mdx#166, @clecat)
- Improve error reporting of ocaml-mdx test (realworldocaml/mdx#172, @Julow)
- Rule: Pass the --section option to `test` (realworldocaml/mdx#176, @Julow)
- Remove trailing whitespaces from shell outputs and toplevel evals (realworldocaml/mdx#166, @clecat)
- Remove inappropriate empty lines in generated dune rules (realworldocaml/mdx#163, @Julow)
- Fix ignored `skip` label in `ocaml-mdx pp` (realworldocaml/mdx#1561, @craigfe)
- Fix synchronization of new parts from markdown to `.ml` (realworldocaml/mdx#156, @Julow)
- Fix ignored `[@@@Parts ...]` markers within module definitions (realworldocaml/mdx#155, @Julow)
- Fix a bug in internal OCaml version comparison that lead to crashes in some cases (realworldocaml/mdx#145, @gpetiot)
- Promote to empty `.ml` file when using `to-ml` direction (realworldocaml/mdx#139, @clecat)
- Apply `--force-output` to `.ml` file as well (realworldocaml/mdx#137, @clecat)
- Fix a bug preventing `.corrected` files to be written in some cases (realworldocaml/mdx#136, @clecat)
- Add compatibility with `4.09.0` (realworldocaml/mdx#133, @xclerc)

#### Removed

- Remove the `output` subcommand as it was very specific to RealWorldOCaml needs (realworldocaml/mdx#195, @NathanReb)
- Remove the `infer-timestamp` direction (realworldocaml/mdx#171 @Julow)
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.

Split a submodule into parts with mdx in to-ml mode
2 participants