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

Optimize the --numeric feature for line/region re-indentation in editors #1609

Closed
wants to merge 1 commit into from

Conversation

gpetiot
Copy link
Collaborator

@gpetiot gpetiot commented Feb 23, 2021

Wait for #1639 to be merged so the slicing optimization can be reviewed independently!

This is a reboot of #1207, as the time overhead made the feature clunky, and inspired by #1484 to reduce the time overhead.

We don't need to parse invalid files outside of this feature, so the --format-invalid-files option has been removed. Instead, this PR defines a new --numeric=X-Y option that should have the same semantic as --numeric --lines=X-Y in ocp-indent.

The idea is:

  • if the program is too big (exceeds a number of lines) (as parsing the whole file would be too time consuming):

    • try to slice the input file into a smaller program keeping only the set of structure/signature items encompassing the X--Y range of lines. (note: the implementation of this feature is a bit hackish, but this should have nice results on most input if we consider the input is a file "almost" formatted already)
    • try to parse either the sliced source, the recovered source (if the first one failed, using menhir recovery parsing), or the original source (if both previously failed)
    • if the parsing is successful, we format the AST, then match the location tree of the formatted AST against the location tree of the unformatted AST to get the indentation produced by ocamlformat [1]
    • if the parsing failed, we use the ocp-indent API to produce an indentation
  • if the program is not too big:

    • try to parse either the source, or the recovered source (if the first one failed, using menhir recovery parsing)
    • if the parsing is successful, we format the AST, then match the location tree of the formatted AST against the location tree of the unformatted AST to get the indentation produced by ocamlformat [1]
    • if the parsing failed, we use the ocp-indent API to produce an indentation

Notes:
[1] this does not return optimal results when multiple lines are made into one when formatted, for example "let x =\n a\nin\n ...." will be formatted as a single line "let x = a in\n ..." so this implementation cannot detect this and will return the same indentation for the 3 lines, which will not look good), ocp-indent is called on this range of lines to improve the indentation.

cc @charlesetc @olydis @emillon

@gpetiot gpetiot force-pushed the split-format-indent branch from e021474 to 9c65da5 Compare March 8, 2021 12:44
@gpetiot gpetiot force-pushed the split-format-indent branch 4 times, most recently from a6aa4cc to 6bb1d15 Compare March 23, 2021 09:28
@gpetiot gpetiot force-pushed the split-format-indent branch 4 times, most recently from 9f491fc to 56a34fb Compare March 30, 2021 13:20
@gpetiot gpetiot force-pushed the split-format-indent branch 4 times, most recently from 16a5fd4 to 7283330 Compare April 2, 2021 13:22
@gpetiot gpetiot marked this pull request as ready for review April 8, 2021 20:47
@gpetiot gpetiot requested review from emillon and Julow April 8, 2021 20:47
@gpetiot
Copy link
Collaborator Author

gpetiot commented Apr 8, 2021

This one is finally ready for review @emillon @Julow, and ready to be tested @charlesetc @olydis

@gpetiot gpetiot force-pushed the split-format-indent branch 2 times, most recently from 03b1b6f to 84c8c46 Compare April 9, 2021 12:36
@gpetiot gpetiot force-pushed the split-format-indent branch from 84c8c46 to c422db9 Compare April 22, 2021 10:35
@gpetiot gpetiot marked this pull request as draft April 22, 2021 11:00
@gpetiot gpetiot changed the title Line/region indentation feature for emacs Slicing optimization for line/region indentation feature for emacs Apr 22, 2021
@gpetiot gpetiot force-pushed the split-format-indent branch from c422db9 to da890fe Compare June 1, 2021 09:49
@gpetiot gpetiot marked this pull request as ready for review June 1, 2021 09:49
@gpetiot gpetiot force-pushed the split-format-indent branch from da890fe to 20b36a9 Compare June 24, 2021 12:49
@gpetiot gpetiot force-pushed the split-format-indent branch 2 times, most recently from 817670e to 04a75bc Compare August 4, 2021 17:34
@gpetiot gpetiot force-pushed the split-format-indent branch from 04a75bc to 9f0d8e0 Compare September 1, 2021 09:32
@gpetiot gpetiot marked this pull request as draft September 8, 2021 16:27
@gpetiot gpetiot force-pushed the split-format-indent branch from 9f0d8e0 to 3449dba Compare October 28, 2021 13:09
@gpetiot gpetiot force-pushed the split-format-indent branch 2 times, most recently from cf065a3 to cec76b6 Compare January 18, 2022 13:14
@gpetiot gpetiot marked this pull request as ready for review January 18, 2022 13:18
@gpetiot gpetiot force-pushed the split-format-indent branch from cec76b6 to 267622e Compare January 28, 2022 11:06
@gpetiot gpetiot force-pushed the split-format-indent branch from 267622e to 6cb94d1 Compare January 28, 2022 11:09
@gpetiot gpetiot changed the title Slicing optimization for line/region indentation feature for emacs Optimize the --numeric feature for line/region re-indentation in editors Jan 28, 2022
@gpetiot
Copy link
Collaborator Author

gpetiot commented Aug 24, 2022

This is too ad-hoc. Should use a legit parser instead.

@gpetiot gpetiot closed this Aug 24, 2022
@gpetiot gpetiot deleted the split-format-indent branch August 24, 2022 20:29
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.

2 participants