Skip to content

Feature request: Allow submodule-body indentations other than 2 spaces #1613

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

Open
ELLIOTTCABLE opened this issue Feb 27, 2021 · 10 comments
Open

Comments

@ELLIOTTCABLE
Copy link

Is your feature request related to a problem? Please describe.
At the moment, as far as I can tell, there's no way to reliably produce output like the following:

module Ex = struct
        type t
        (* ... *)
end

Unlike, for instance, extensions, functions, let-bindings, and type-decls, submodule bodies seem to be hard-coded to always be two spaces!

Describe the solution you'd like
A indentation setting that applies to structs and signatures:

// .ocamlformat
extension-indent = 3
function-indent = 3
let-binding-indent = 3
type-decl-indent = 3

module-indent = 3

... producing, for the above:

module Ex = struct
   type t
   (* ... *)
end
@gpetiot
Copy link
Collaborator

gpetiot commented Mar 4, 2021

Hi, my personal opinion about this kind of options is that it was a mistake to add them in ocamlformat in the first place, as it adds noise in the code, and it goes against the purpose of ocamlformat to promote a normalized formatting for the code of the community.
There will always be some place where we forgot to create an option to customize the indentation, we could argue that it would make sense to have for consistency but it reduces the chance to have a normalized style one day, I think.

I was personally considering removing these options for the 1.0 release (note that removing more and more options to progressively converge towards a unique style is our goal working towards the 1.0 release, and these indentation-options are the low hanging fruits of this simplification).

Out of curiosity, why do you want to apply a 3-column indentation instead of 2? (if I'm not mistaken it seems that most people are happy with 2). Would removing these options be an issue for your project(s)?

@ELLIOTTCABLE
Copy link
Author

Well, there's kinda three points to make here:

  1. Regarding options in general, I 100% agree with you. I'm all for the Prettier approach: convention over configuration for style stuff, so we're not simply promoting style-nitpicking discussions from the source-files up to the config-file, but instead removing them entirely. Not here to argue against that! Fantastic plan!

  2. Regarding spacing in specific, and I think this is the most relevant point — notice that Prettier, as well as all the other tools I'm aware of, do continue to offer configuration for spacing, even after having removed almost all other configuration. I'd posit that there's several upsides to this that make it worth an exception:

    • Users will generally have an editor-wide spacing configuration for all languages and projects (mostly, either 2, 4, or 8 spaces; although for reasons below, I don't personally think it's a good idea to hard-code support for those specific values); and no matter what "default" value you pick, your autoformatting tooling is going to then be "fighting" some users' editors.

      The consequences of that may be subtle, and vary between editors, but could include anything from "hitting return triggers the editor's auto-indentation, moving the cursor to column X; but then hitting a printable character triggers the editor's auto-formatting, which feeds through ocamlformat and moves that character to column Y instead" (and that happening on every. single. line. return.) up to "the editor reindents lines after a linereturn, overriding the ocamlformat indentation."

    • If a "project" is multilingual, it may not even be possible to simply "do what ocamlformat likes", and configure the .editorconfig to twosp indentation — what if there's another tool that dictates foursp, or eightsp; and it also takes the route you're suggesting? Now it's completely impossible to unify the indentation across the whole project. (Not that that's a huge, critical problem, or anything; but it does rather complicate editor-configuration for all contributors ...)

    • I can't personally predict every nasty interaction here, but one way to look at it is that this is the Most Bikesheddable By Tooling thing across all code: pretty much every other code-style concept is at least somewhat language-specific, and thus most tooling will be agnostic to the possible choices; but indentation, specifically, is something that almost any tool can Have An Opinion About and then apply that opinion to all source-code. Thus, I think having the ability to configure all those tools to not constantly fight one-another is a very, very good thing.

  3. And if you're asking about threesp, well, that's a personal taste thing. Pretty much every discussion about this topic hits the same points of discussion: two spaces isn't enough visual differentiation for quickly skimming code, especially at small point-sizes … but four spaces is just too much, and wastes line-space within the limited 80 characters available, lengthening code vertically and making annoyingly/messily-broken lines slightly more likely. And yet somehow those discussions never hit on the solution of compromising? I don't know why programmers are so obsessed with powers-of-two when memorylayout isn't involved?

    IMHO, 3 is the perfect compromise between 2 and 4 spaces; I've been using it happily in all of my code for a decade. Doesn't seem very relevant to this issue, though; it's just a matter of my own taste.

Hope some of those thoughts would be useful! I'm not super-strongly opinionated about any of them, but it does seem like — if any options are to be kept — then the general, not-OCaml-specific ones are the most important options to "bend" the no-options rule on. (Specifically, I posit that it'd be good practice to allow interoperability with anything .editorconfig files can configure.)

@gpetiot
Copy link
Collaborator

gpetiot commented Nov 4, 2021

Sorry for the late update, my position didn't really change in the meantime, we still avoid adding new options, and so far most people seem happy with the current indentation of items. So we're unlikely to change it for now but we might reconsider if there is a stronger push from the community (we want to reflect the most commonly used styles and conventions and not focus on having something very customizable). I'm thus closing this issue :)

@gpetiot gpetiot closed this as completed Nov 4, 2021
@ELLIOTTCABLE
Copy link
Author

Okay, I understand the constraints as a fellow maintainer.

Here's hoping I'm not the only OCamleer who thinks 2 is a ridiculously small amount of space, though. Awaiting others' input with bated breath!

@toastal
Copy link

toastal commented Nov 17, 2022

Coming to stir the pot… ;)

In the light of prettier very much considering using tabs next version for accessibility purposes, I would love to see tabs supported (so indentation is configurable on the user’s end).

Anecdotally, as I’ve aged 2-spaced code has become harder and harder to read—and I used to be a proponent of 2-space indentation as a default 4 or so years ago. 2-space indentation is the default in the PureScript community and the compiler there doesn’t support tabs. Like the OP I prefer 3 or 4-spaced code personally as beyond 2 is where the indentation contrast actually ‘clicks’ and I can read the code a lot faster without the need to scrutinize. I recently picked up some OCaml to try a new language and had been using tabs since it’s supported by the compiler. I’d like to consider the language beyond a one-off, for-fun scripts so I went to find the community formatter, but I ended up in this issue because common code uses this formatter and is 2-spaced and difficult for me to read.

@toastal
Copy link

toastal commented Feb 23, 2023

If anyone is looking for a tab-support workaround in a Nix shell, here is a minimal-but-expandable Flake that can be used as a starter:

{
  inputs = {
    nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
  };

  outputs = { self, nixpkgs }@inputs:
    let
      supportedSystems = [ "x86_64-linux" "aarch64-linux" ];

      forAllSystems = f: nixpkgs.lib.genAttrs supportedSystems (system: f system);

      nixpkgsFor = forAllSystems (system: import nixpkgs {
        inherit system;
        overlays = [ self.overlay ];
      });
    in
    {
      overlay = final: prev: {
        ocamlformat-with-tabs = prev.writeShellScriptBin "ocamlformat" /* sh */ ''
           set -euo pipefail
           exec ${prev.lib.getExe prev.ocamlformat} "$@" | unexpand --first-only --tabs=2
        '';
      };

      devShell = forAllSystems (system:
        let
          pkgs = nixpkgsFor.${system};
        in
        pkgs.mkShell {
          buildInputs = with pkgs; [
            ocamlformat-with-tabs
          ];
        });
    };
}

(irony of this code being 2-space hasn’t escaped me but the same issue of 2-space code is in the Nix formatters too)

This will put a wrapped ocamlformat binary on one's $PATH which pipes to unexpand from coreutils to turn 2-space indentation (the defaults) into tabs. Editors, like Neovim ran from the Nix shell, can pick up on this and will reformat the ocamlformat content with tabs. This is a good way to use tabs in a specific project and not mess with others.

@Julow
Copy link
Collaborator

Julow commented May 22, 2023

This seems popular and have been closed too quickly, re-opening.

Reducing the number of option is a goal but we also want to keep the reasonable options that people care about.

@Julow Julow reopened this May 22, 2023
@MrDaiki
Copy link

MrDaiki commented Nov 4, 2024

Little ping to see the state of this issue. Wouldn't it be better to add parameter for deciding indent size for all the project ?

The main problem with how ocamlformat handle indent size is that you must redifine the indent size for all types of elements (case-exp, extension, function ...). Wouldn't it be better to define a default tab size behavior, and then to be able to customize it for specific items ?

@toastal
Copy link

toastal commented Nov 4, 2024

It’s also worth noting that Topiary allows one to configure indentation & supports OCaml.

1

Footnotes

  1. Please consider giving up MS GitHub or offering a non-proprietary, non-US-corporate-controlled mirror for this free software project. I wish to delete this Microsoft account in the future, but I need more projects like this to support alternative methods to send patches & contribute.

@HPRIOR
Copy link

HPRIOR commented Apr 20, 2025

I'd like to see more options for indentation. I have created a feature request here (prior to seeing this one) and started working on a PR.

Little ping to see the state of this issue. Wouldn't it be better to add parameter for deciding indent size for all the project ?

The main problem with how ocamlformat handle indent size is that you must redifine the indent size for all types of elements (case-exp, extension, function ...). Wouldn't it be better to define a default tab size behavior, and then to be able to customize it for specific items ?

As mentioned in my request, since modules are ocamls overarching programming unit it's quite hard to decide which syntax elements should be subject to a 'module-indent' option. I have proposed supporting something like configurable indent categories to replace the existing hard coded values:

--indent-small=1
--indent-base=2
--indent-medium=3
--indent-large=4

As far as I can tell, this would be fairly straightforward to implement.

Where there are hard coded values, these would simply be replaced by configuration:

...
  | Lapply (li1, li2) ->
      hvbox c.conf.fmt_opts.base_indent.v
        ( fmt_longident li1
        $ wrap (cut_break $ str "(") (str ")") (fmt_longident li2) )
...

Or if more fine grained compatibility with existing indentation options is needed (assuming the existing indent options are consumed as Options):

let box =
  if c.conf.fmt_opts.ocp_indent_compat.v then
      match pld with
      | PStr [{pstr_desc= Pstr_eval _; _}] | PTyp _ | PPat _ ->
          hvbox (c.conf.fmt_opts.extension_indent.v |> Option.value c.conf.fmt_opts.base_indent.v) 
      | PSig _ | PStr _ ->
          hvbox (c.conf.fmt_opts.stritem_extension_indent.v |> Option.value c.conf.fmt_opts.base_indent.v)
  else Fn.id

I would be more than happy to implement this if supported by the maintainers @gpetiot @Julow.

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

No branches or pull requests

6 participants