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

Jump to bottom

Feature request: Update default profile to use indicate-multiline-delimiters = closing-on-separate-line #1958

Open
hcarty opened this issue Dec 10, 2021 · 8 comments · May be fixed by #2020
Open

Feature request: Update default profile to use indicate-multiline-delimiters = closing-on-separate-line #1958

hcarty opened this issue Dec 10, 2021 · 8 comments · May be fixed by #2020
Labels
Kind/feature-request

Comments

@hcarty
Copy link
Contributor

hcarty commented Dec 10, 2021 •
edited
Loading

Is your feature request related to a problem? Please describe.

The indicate-multiline-delimiters = closing-on-separate-line setting was deprecated in ocamlformat 0.20.0. I've personally found this feature very useful and had good feedback from other developers on the same codebase after it was introduced. In particular, it makes modifying anything with a closing delimiter easier because the last line of the delimited body is no longer special. This also makes diffs easier to understand as the )/]/} no longer clutters diffs when the delimited body changes.

Describe the solution you'd like

I would like to have indicate-multiline-delimiters = closing-on-separate-line as part of the default profile. As @gpetiot pointed out to me, this will cause a lot of churn for users of the default profile! So this seems like it's worth considering before a 1.0.0 release.

Additional context

Issues:

PRs:
@vphantom vphantom mentioned this issue Jan 28, 2022
Closed
@hcarty
Copy link
Contributor Author

hcarty commented Feb 10, 2022

A gentle ping - is there any chance of this happening? At work we've been sticking with 0.19.0 in large part because of this deprecation and I've done the same with personal projects.

@gpetiot
Copy link
Collaborator

gpetiot commented Feb 10, 2022

Hi, sorry for the late reply.

I don't know if this option will be part of the default profile, this profile is driven by the maintainers of the mirage and the ocaml platform projects. @samoht @tmattio what do you think of having this setting by default?

For the record I like this option as well, in other issues I mentioned that I would like to put together a profile that would be more diff-friendly than the default one, that would vertically unfold most of the code (kinda like the deprecated sparse profile without the unnecessary line breaks). And such profile would use this setting.
Most deprecated options won't disappear overnight, my plan is to have most users happy with the available profiles before removing the options, so you can safely use them in the meantime.

@hcarty
Copy link
Contributor Author

hcarty commented Feb 13, 2022

Thank you @gpetiot - I've seen your comments about wanting a more diff-friendly format and I agree!

Would it be more helpful for me to reframe this issue as a request for a more diff-friendly or newcomer friendly version of the default profile? I could also open a new issue for that purpose. I don't want to add issue noise though if that wouldn't be helpful for you.

@gpetiot
Copy link
Collaborator

gpetiot commented Feb 14, 2022

Would it be more helpful for me to reframe this issue as a request for a more diff-friendly or newcomer friendly version of the default profile? I could also open a new issue for that purpose. I don't want to add issue noise though if that wouldn't be helpful for you.

I don't think it's necessary to open an issue, I will get working on it soon and you can give feedback once the PR is open.

@hcarty
Copy link
Contributor Author

hcarty commented Feb 16, 2022

Sounds good - thank you!

@gpetiot gpetiot linked a pull request Feb 23, 2022 that will close this issue
Draft
@gpetiot
Copy link
Collaborator

gpetiot commented Feb 23, 2022

I opened #2020, don't hesitate to give some feedback if you have time to give it a try!

@hcarty
Copy link
Contributor Author

hcarty commented Feb 24, 2022

Thank you! I'll give it a try this week or next.

@tmattio
Copy link
Contributor

tmattio commented Mar 12, 2022

I just tried the diff-friendly profile and left some feedback on the PR.

I don't know if this option will be part of the default profile, this profile is driven by the maintainers of the mirage and the ocaml platform projects. @samoht @tmattio what do you think of having this setting by default?

IMHO, it's a good idea to have a default profile that is driven independently of any project or preference. I could definitely see this diff-friendly profile becoming the default and creating a mirage profile from the current default one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Kind/feature-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Define a diff-friendly profile gpetiot/ocamlformat
3 participants
@hcarty @gpetiot @tmattio