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

Add settings for formatting #112

Open
wants to merge 4 commits into
base: master
from
Open

Add settings for formatting #112

wants to merge 4 commits into from

Conversation

@50Wliu
Copy link
Member

@50Wliu 50Wliu commented Jun 19, 2019

Makes all the formatting settings from the LS configurable 🎉. And now that we have this additional level of configuration, I figure it's a good time to hook into the format-on-type and format-on-save providers. On-type can be disabled within ide-java, and on-save can be disabled via atom-ide-ui.

I still need to figure out how to make this a smooth transition, though, as I'm sure most projects aren't following Eclipse's default formatting rules.

Fixes #109

50Wliu added 4 commits Jun 19, 2019
For new formatting options
@50Wliu
Copy link
Member Author

@50Wliu 50Wliu commented Jun 20, 2019

I see a few options here:

  1. Add an option to ide-java that controls on-save formatting, and default both that one and the on-type formatting to off. (Note that the on-save option would be in a different section, which would be weird)
  2. Add a notification that pops up on upgrade letting people know about the new formatting behavior and how to customize it.
  3. Don't add on-save/on-type formatting. Keep it range/file-based only.
  4. Only enable on-save/on-type formatting if a URL is provided in settings.
  5. Something else?
@calebmeyer
Copy link

@calebmeyer calebmeyer commented Jun 20, 2019

I like the popup. Until atom has some place a person can go to look at settings across packages, it's going to be hard to discover these settings. I also think it would be okay to have a short mention of the on-save setting in the README.

@mdiin
Copy link
Contributor

@mdiin mdiin commented Feb 21, 2020

I see this includes the change in #116. Any chance of getting one of these merged?

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

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.