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

Editorconfig page #2427

Merged
merged 37 commits into from
Aug 29, 2022
Merged

Editorconfig page #2427

merged 37 commits into from
Aug 29, 2022

Conversation

alanlomeli
Copy link
Member

@alanlomeli alanlomeli commented Aug 16, 2022

Action Items

Configuration introduction

  • Grab a good understanding of both practical and theoretical of how this config works with both IDE and CLI.
  • Write an introductory paragraph on how .editorconfig works.
  • Show some initial examples on how to use it.
  • Explain how the IDE should respect the settings but to be caution because that this IDE-dependent.
  • Make enfasis with IDE’s with UI
  • Mention the online tool for trying different configs.
  • Add a gif of the online tool.
  • Add an introductory description to each category.

Categorization and recommendation system

  • Convert current page to .fsx file.
  • Add new category headers
  • Assign each setting to it's own category
  • Add custom tooltip for the category section icon that shows a little explanation when the user hovers the icon.
  • Design a recommendation system for 3 types of settings: Good to use, not recommended, and the ones to not use.
  • Add a G-Research section to the recommendation.
  • Before the actual settings, redact an explanation of the recommendation system to the user, what the different icons mean (or color, symbol, etc depending on what we use), and the reason why they can use it or why they shouldn’t.
  • Create web components using lit library for recommendation icons.
  • Create web components using haunted library.
  • Implement the recommendation system for each setting.
  • Describe each setting.
  • Make sure each setting is searchable and if not see what we might be missing.
  • Generate appropriate code for each setting.

@nojaf
Copy link
Contributor

nojaf commented Aug 16, 2022

For the initial introduction you want to:

  • Clarify we use .editorconfig and elaborate on its usage, showing some initial examples.
[*.fs]
fsharp_space_before_uppercase_invocation = true

# Write a comment by starting the line with a '#'
[*.{fs,fsx,fsi}]
fsharp_bar_before_discriminated_union_declaration = true

# Apply specific settings for a targeted subfolder
[src/Elmish/View.fs]
fsharp_multiline_block_brackets_on_same_column = true
fsharp_experimental_stroustrup_style = true
  • Your IDE should respect these same settings, however, the implementation of that is editor specific. The same goes for whether the IDE has a UI for the settings or not.

  • Mention that people can quickly try settings from our online tool and use the Copy settings button. Perhaps insert a gif of that.

Add new configuration file
Temporarily deleted settings for ramping up the new configuration page.
- Redacted basic introduction
- Included gif for online tool usage.
- Create section where we explain each recommendation (to be redacted)
- Add icons for each recommendation
- Implement tooltip for when users hover the symbol
Copy link
Contributor

@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.

So far, so good.

docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
@nojaf
Copy link
Contributor

nojaf commented Aug 22, 2022

A first breakdown of the settings:

Auxiliary settings

setting category reason / thoughts
indent_size orange Both style guides are based on 4
max_line_length green
end_of_line green Recommendation: use lf regardless of OS
insert_final_newline green Recommendation: enable, a file should end on a newline character
fsharp_space_before_parameter orange No guide changes default
fsharp_space_before_lowercase_invocation orange No guide changes default
fsharp_space_before_uppercase_invocation green, GR Note isn't always respected, in some cases it can lead to invalid code
fsharp_space_before_class_constructor orange No guide changes default
fsharp_space_before_member green, GR
fsharp_space_before_colon green, GR
fsharp_space_after_comma orange No guide changes default
fsharp_space_before_semicolon green, GR
fsharp_space_after_semicolon orange No guide changes default
fsharp_space_around_delimiter orange No guide changes default

Maximum width constraints

setting category reason / thoughts
fsharp_max_if_then_short_width green Agree within team
fsharp_max_if_then_else_short_width green Agree within team
fsharp_max_infix_operator_expression green Agree within team
fsharp_max_record_width green Agree within team
fsharp_max_record_number_of_items green Agree within team
fsharp_record_multiline_formatter green Agree within team
fsharp_max_array_or_list_width green Agree within team
fsharp_max_array_or_list_number_of_items green Agree within team
fsharp_array_or_list_multiline_formatter green Agree within team
fsharp_max_value_binding_width green Agree within team
fsharp_max_function_binding_width green Agree within team
fsharp_max_dot_get_expression_width green Agree within team

G-Research style

setting category reason / thoughts
fsharp_multiline_block_brackets_on_same_column green, GR
fsharp_newline_between_type_definition_and_members green, GR
fsharp_align_function_signature_to_indentation green, GR
fsharp_alternative_long_member_definitions green, GR
fsharp_multi_line_lambda_closing_newline green, GR
fsharp_experimental_keep_indent_in_branch orange, GR Still experimental
fsharp_blank_lines_around_nested_multiline_expressions green, GR
fsharp_bar_before_discriminated_union_declaration green, GR

Other

setting category reason / thoughts
fsharp_experimental_stroustrup_style orange Not part of any guide
fsharp_keep_max_number_of_blank_lines green I recommend 1
fsharp_strict_mode red Never use this please

There are a couple of Auxiliary settings where the G-Research guide will change the defaults.
But these features weren't really implemented as complete stylistic alternatives. They fall more under Auxiliary than G-Research style but should still get the G icon to indicate that the G-Research guide changes the default value of them.


(**
## <img class="gresearch-recommendation align-top" data-bs-toggle="tooltip" data-bs-custom-class="gresearch-tooltip" data-bs-title="If you use one of these you should use all G-Research settings for consistency reasons" data-bs-custom-class="orange-tooltip" src="{{root}}/gresearch.svg" alt="G-Research logo"/> G-Research style
#### <i class="bi bi-check-circle-fill orange-recommendation" data-bs-toggle="tooltip" data-bs-custom-class="orange-tooltip" data-bs-title="This setting is not recommended"></i> indent_size
Copy link
Contributor

Choose a reason for hiding this comment

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

@KenBonny don't mind the fsx part, we are using fsdocs.
Everything in between (** and *) will be rendered as Markdown.

We currently need to write a lot of HTML code inside our markdown to show the right icon with a tooltip.
Using a web component, we can wrap all that.
We are most likely going to go with lit.
Cook some components up and then place the HTML with something like <fantomas-icon></fantomas-icon>.

docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
(*** include-it ***)

(**
## Maximum width constraints
Copy link
Contributor

Choose a reason for hiding this comment

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

Before listing all the settings, I would add some introduction text that summarizes what these settings do. You can use some lorem ipsum for now if nothing comes to mind.

docs/.style/fsdocs-custom.sass Outdated Show resolved Hide resolved
- Deleted repeated G-Research icon
- Changed output background stylings
- Deleted 'default_config' keyword
- Temporal lorep ipsum that later will explain sections
- Imported Lit dependency for web components
- Created 4 web componentes for each recommendation system
- Changed hardcoded icons to custom web components
- Changed icons stylings for allowing different stylings depending on
  icon location
Copy link
Contributor

@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.

Could you please also update the .README file in docs.
eval should be passed in as argument to get the script working.

docs/content/fantomas-web-components.js Outdated Show resolved Hide resolved
docs/content/fantomas-web-components.js Outdated Show resolved Hide resolved
docs/content/fantomas-web-components.js Outdated Show resolved Hide resolved
docs/content/fantomas-web-components.js Outdated Show resolved Hide resolved
docs/.style/variables.sass Outdated Show resolved Hide resolved
docs/_template.html Outdated Show resolved Hide resolved
@nojaf
Copy link
Contributor

nojaf commented Aug 24, 2022

One other interesting thing I discovered was the haunted project.
This allows you to remove some of the boilerplate:

import { html } from 'https://cdn.skypack.dev/lit';
import { component } from 'https://cdn.skypack.dev/haunted';

function SeaGreenCategory({ tooltip }) {
    return  html`<i class="bi bi-check-circle-fill green-recommendation" style="color: seagreen" 
                    data-bs-toggle="tooltip" data-bs-custom-class="green-tooltip" data-bs-title="${tooltip}"></i>`;
}

customElements.define('fantomas-seagreen', component(SeaGreenCategory, { useShadowDOM: false,
                                                                               observedAttributes: [ 'tooltip'] }));

Copy link
Contributor

@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.

Move the gresearch svg to the images folder.
Update svg to have a width and height of 14px and change the padding to be 3px instead of 4px. That way the image will have a total width and height of 20px which will better match the bootstrap icon size.

docs/.style/variables.sass Outdated Show resolved Hide resolved
docs/content/fantomas-setting-icon.js Outdated Show resolved Hide resolved
docs/content/fantomas-setting-icon.js Outdated Show resolved Hide resolved
docs/content/fantomas-setting-icon.js Outdated Show resolved Hide resolved
docs/content/fantomas-setting-icon.js Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Outdated Show resolved Hide resolved
docs/docs/end-users/Configuration.fsx Show resolved Hide resolved
- Add eval flag for readme file
- Refactor code for better readability
- Deleted unnecesary sass vars
- Reworded settings redactions
- Resize SVG icon
@nojaf
Copy link
Contributor

nojaf commented Aug 28, 2022

I took a glimpse at the last commits and I can still several previous remarks that are not yet addressed. Please circle back through each and every remake and finalize this PR.

@alanlomeli
Copy link
Member Author

Move the gresearch svg to the images folder. Update svg to have a width and height of 14px and change the padding to be 3px instead of 4px. That way the image will have a total width and height of 20px which will better match the bootstrap icon size.

With 14px, the icon was looking kinda small, so I set it to 15px. Hope that works.
imagen

Copy link
Contributor

@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.

Alright, let's merge this in. I'll address some things in a next PR.

@nojaf nojaf marked this pull request as ready for review August 29, 2022 06:38
@nojaf nojaf merged commit b586014 into fsprojects:master Aug 29, 2022
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