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 button to import settings from .editorconfig file #615

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

josh-degraw
Copy link

This adds an input to try and load settings directly from an editorconfig file to simplify the repro process. Something is still off (I think it's related to the order param on the options) because it doesn't always load properly but it's close.

@josh-degraw josh-degraw requested a review from nojaf November 11, 2023 22:02
@josh-degraw josh-degraw self-assigned this Nov 11, 2023
Copy link
Collaborator

@nojaf nojaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As expressed in #436, I'm not really a fan of this feature.

It will lead to users reporting problems where dozens of settings will have no impact on the actual problem they are trying to report. I'm not at all motivated to triage issues where the bug wasn't reproduced with the smallest possible sample.

This on a practical side will also lead to long URLs which GitHub won't support for the issue creation.

Could we find some middle ground and work out some sort of two-way binding system with the settings UI? Maybe we add a textarea to paste your relevant editor config settings. If the text changes, we update the settings in the Model. If the settings change via the UI, we update the editorconfig text.

"resolved": "6.0.0",
"contentHash": "/iUeP3tq1S0XdNNoMz5C9twLSrM/TH+qElHkXWaPvuNOt+99G75NrV0OS2EqHx5wMN7popYjpc8oTjC1y16DLg=="
},
"fantomas.core": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required?

| _ -> failwithf $"Cannot decode `{line}`"

let decodeOptionsFromEditorConfigFile (currentSettings: Map<string, FantomasOption>) (fileText: string) =
fileText.Split([| Environment.NewLine |], StringSplitOptions.RemoveEmptyEntries)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't quite work when there are multiple sections.
This will assume all settings found will apply.
Might be confusing for the users.

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