Support Rufo for formatting #295

Activity

1. wingrunr21

   added

   feature-requestAdds currently unsupported functionalityAdds currently unsupported functionality

   on Mar 13, 2018
2. gurgeous commented on Mar 13, 2018

   

   gurgeous

   on Mar 13, 2018

   Contributor

   More actions

   Per discussion in #285, there are two ways to implement this:

   1. Add support for ruby.format = rubocop|rufo|false.
   2. Add support for ruby.format = rubocop|false, and encourage people to use vscode-rufo for rufo support.

   (In both cases ruby.format will default to rubocop if ruby.lint.rubocop is set)

   Personally I'd opt for (2) since I think it's good practice to utilize other extensions and minimize our own codebase. But I don't have a strong opinion. How should I proceed?
3. gurgeous commented on Mar 13, 2018

   

   gurgeous

   on Mar 13, 2018

   Contributor

   More actions

   I'll start with (2) until someone tells me otherwise. I'll work on top of #297 since this is also related to options processing.
4. gurgeous commented on Mar 13, 2018

   

   gurgeous

   on Mar 13, 2018

   Contributor

   More actions

   Any preference between false, 'none', and 'other'? Maybe if we can agree here we can apply it to other parts of the config. I'm just asking because @castwide proposed 'other', we have some existing code that uses 'none', and in my ignorance I suggested false. :)
5. castwide commented on Mar 13, 2018

   

   castwide

   on Mar 13, 2018

   Contributor

   More actions

   I don't have any particular preference, other than standardizing around a common convention. false might be a good compromise, since it covers the meaning of both other (I get this feature from somewhere else) and none (I don't want this feature at all).
6. gurgeous commented on Mar 13, 2018

   

   gurgeous

   on Mar 13, 2018

   Contributor

   More actions

   Working on this now. I'll use false.

   Another tidbit - I believe the current implementation ALWAYS attempts to format with rubocop (regardless of settings). That was acceptable since it failed so quietly. Now that we have loud error messages, I think we should make it so that you have to opt into formatting by setting ruby.format to rubocop. Existing users will need to make a config change to re-enable formatting. I don't think this is a huge deal since I bet that rubocop formatting is broken for most existing users due to the vscode timeout issue.
7. castwide commented on Mar 13, 2018

   

   castwide

   on Mar 13, 2018

   Contributor

   More actions

   While experimenting with configuration options for ruby.codeCompletion and ruby.intellisense, I ran into a couple of issues with creating a false option. The first thing I tried was adding false to the configuration's enum. If the configuration's type is string, setting it to false on the Settings page reports a type error. If you set the type to ["string", "boolean"], the Settings page adds true to the completion options, even though true is not in the enum.

   A couple possible solutions:

   • Use the string "false" instead of the value false (if we do this, we might be able to find a better word to use)
   • Use an empty string or null

   @gurgeous Any thoughts?
8. castwide commented on Mar 13, 2018

   

   castwide

   on Mar 13, 2018

   Last edited by castwide

   Contributor

   More actions

   Well, this is interesting. Out of curiosity, I tried to set the configuration's type to ["string", "false"], and it worked.

   Edit: Spoke too soon. Selecting false still gets marked as an error.
9. gurgeous commented on Mar 14, 2018

   

   gurgeous

   on Mar 14, 2018

   Contributor

   More actions

   In my PR I use this:

           "ruby.format": {
             "type": [ "boolean", "string" ],
             "enum": [ false, "rubocop" ],
             "default": false,
             "description": "Which system to use for formatting, or false for no formatting"
           }

   and it correctly flags true as invalid:

   

   Seems fine, I think. Does anyone disagree? I don't feel strongly about any of these options.
10. castwide commented on Mar 14, 2018

    

    castwide

    on Mar 14, 2018

    Contributor

    More actions

    I used a similar configuration for ruby.codeCompletion:

    "ruby.codeCompletion": {
      "type": ["boolean", "string"],
      "enum": [false, "rcodetools"],
      "default": false,
      "description": "Method to use for code completion. Use `false` if another extension provides this feature."
    }
    

    It flags true the same way, which is fine. But it also adds true to the completion items:

    
11. gurgeous commented on Mar 14, 2018

    

    gurgeous

    on Mar 14, 2018

    Contributor

    More actions

    Huh. Completion presents it as an option. I hadn't noticed that. That little Edit pencil thing on the left seems to use the enum (doesn't show true), and the green squiggles are working properly...

    @wingrunr21, what do you think? I really don't care either way.
12. castwide commented on Mar 15, 2018

    

    castwide

    on Mar 15, 2018

    Contributor

    More actions

    Microsoft tagged the completion item issue as a bug and added it to their backlog, so we should be okay to keep false as an option.
13. gurgeous commented on Mar 15, 2018

    

    gurgeous

    on Mar 15, 2018

    Contributor

    More actions

    Great! @wingrunr21 or @rebornix - can this be merged?
14. wingrunr21

    closed this as completedin #298on Mar 27, 2018