Add Command to Reformat Current Document

[sanssecours]

The command “Reformat Document” formats the current document using the --auto-correct option of RuboCop. It also shows information about the formatting status in a floating tooltip. The command displays the information about the formatting status either as black text only, or as colorful text if aha is accessible via $PATH.

[jacob-carlborg]

The implementation of this doesn't look good at all:

• No assumption of the path to commands (RuboCop, aha) should be made
• The command should be executed in the same directory as the file to be formatted to be able to pick up any local RuboCop configuration
• As far as I understand, any commands that don't use HTML as the output format will block the editor. I've tried to implement this myself and it took way too long time to finish executing the command to be acceptable
• The command should handle the result from RuboCop much better than this. That is, parse the result and properly display warning/error messages in the HTML view with links back to the source code and/or using the gutter view to display the message

This is what I would expect from a command like this:

• An interface looking like this RuboCop specific bundle [1]. Should only be necessary to show the HTML view if RuboCop reports any issues
• Support for RVM
• Support for Bundler
• Auto detect the path to RuboCop. If the project is using Bundler and the Gemfile contains RuboCop, that version should be used. Otherwise it should be fallback to an environment variable, i.e. TM_RuboCop, or if that doesn't exist, whatever is in PATH
• Not sure if it's possible to start a daemonized command that can return the result back to the editor. If that works it could solve the problem with blocking the editor

The bundle [1] I've already mentioned already contains most of this setup, together with my pull request [2].

[1] https://github.com/mrdougal/textmate2-rubocop [2] https://github.com/mrdougal/textmate2-rubocop/pull/11

[sanssecours]

Hi Jacob,

• No assumption of the path to commands (RuboCop, aha) should be made

what exactly do you mean? The command does not use any absolute paths. It just assumes that rubocop is located in /usr/bin or in any of the locations specified in PATH.

• The command should be executed in the same directory as the file to be formatted to be able to pick up any local RuboCop configuration

As far as I can tell RuboCop determines the configuration for the current file according to the given filepath. You can test this yourself:

1. Create a file ~/Downloads/test.rb with the following content:

# René
puts 'Hi'

1. Run RuboCop on the file:

rubocop ~/Downloads/test.rb

1. Rubocop will print something like this:

Inspecting 1 file
C

Offenses:

test.rb:1:6: C: Use only ascii symbols in comments.
# René
     ^

1 file inspected, 1 offense detected

1. If you create a file ~/Downloads/.rubocop.yml with the following content:

AsciiComments:
  Enabled: false

, then calling rubocop ~/Downloads/test.rb will output the following:

Inspecting 1 file
.

1 file inspected, no offenses detected

• As far as I understand, any commands that don't use HTML as the output format will block the editor. I've tried to implement this myself and it took way too long time to finish executing the command to be acceptable.

While I think you are right, the short delay to reformat the document is acceptable to me. Especially since I do not want to edit a document that RuboCop currently reformats anyway.

• The command should handle the result from RuboCop much better than this. That is, parse the result and properly display warning/error messages in the HTML view with links back to the source code and/or using the gutter view to display the message.

The command reformats the current document anyway. In the best case the warnings and error messages should be gone after RuboCop is finished.

This is what I would expect from a command like this:

An interface looking like this RuboCop specific bundle [1]. Should only be necessary to show the HTML view if RuboCop reports any issues

I think this is more or less a personal preference ☺️. I like to view the information about the warning/errors in the document, even if there are none. And I do not want the command to open the HTML view, since I need to close it afterwards.

• Support for RVM • Support for Bundler

Auto detect the path to RuboCop. If the project is using Bundler and the Gemfile contains RuboCop, that version should be used.

Sorry, I do not use any of those version managers. If you like, maybe you could add support for them.

Otherwise it should be fallback to an environment variable, i.e. TM_RuboCop, or if that doesn't exist, whatever is in PATH

Using an environment variable to specify the location might be a nice extension, although I would probably argue for the name TM_RUBOCOP. TM_RuboCop just does not look right especially since we usually use names like these :

• TM_HG_EXT_DIFF • TM_MARKDOWN • TM_PYCHECKER • TM_SVN_DIFF_CMD • TM_TERMINAL • …

• Not sure if it's possible to start a daemonized command that can return the result back to the editor. If that works it could solve the problem with blocking the editor

That might also be a nice addition. But like I said before, I like the current status of the command. Of course there is always the possibility of improvement. But I like to leave that for another (less lazy 😪) person.

The bundle [1] I've already mentioned already contains most of this setup, together with my pull request [2].

[1] https://github.com/mrdougal/textmate2-rubocop [2] mrdougal/textmate2-rubocop#11

I used the bundle you mentioned too. However it stopped working for me sometimes ago. If I clone your version of the bundle and run “Analyse current file” I get the following error message 😢:

/Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/lib/mate/proxy.rb:26:in `run!': uninitialized constant Proxy::Rubocop (NameError)
    from /Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/lib/mate/proxy.rb:14:in `run!'
    from /Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/lib/mate/runner.rb:23:in `block in run'
    from /Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/lib/mate/runner.rb:22:in `chdir'
    from /Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/lib/mate/runner.rb:22:in `run'
    from /Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/lib/mate/runner.rb:12:in `current_file'
    from /Users/rene/Library/Application Support/Avian/Bundles/textmate2-rubocop.tmbundle/Support/bin/analyse_current_file:5:in `<main>'

Kind regards, René

[jacob-carlborg]

what exactly do you mean? The command does not use any absolute paths. It just assumes that rubocop is located in /usr/bin or in any of the locations specified in PATH.

"Note that while you may already have set the PATH variable for your shell (e.g. bash) it is not inherited in TextMate, as TextMate does not parse your shell setup scripts." [1]. The diff also contains /usr/bin/rubocop in requiredCommands, not really sure what that is. But since OS X 10.11 you don't have write access to /usr/bin. So there's no point in looking for something there which doesn't ship with the OS.

As far as I can tell RuboCop determines the configuration for the current file according to the given filepath

Aha, I though it was based on the current working directory.

While I think you are right, the short delay to reformat the document is acceptable to me

It might not be acceptable for others.

Especially since I do not want to edit a document that RuboCop currently reformats anyway.

No, but the whole editor freezes. You cannot select text, switch tabs, nothing.

The command reformats the current document anyway. In the best case the warnings and error messages should be gone after RuboCop is finished.

There are a lot of things RuboCop warns about that it cannot autocorrect.

I think this is more or less a personal preference ☺️. I like to view the information about the warning/errors in the document, even if there are none.

How would you do that if there are none?

And I do not want the command to open the HTML view, since I need to close it afterwards.

Well, ideally it should only open it if there are any warnings it could not autocorrect.

although I would probably argue for the name TM_RUBOCOP. TM_RuboCop just does not look right

Fair enough.

I used the bundle you mentioned too. However it stopped working for me sometimes ago. If I clone your version of the bundle and run “Analyse current file” I get the following error message

You need my PR [2] 😃.

Personally I don't think this command is generic and well implement enough to be added upstream.

[1] http://blog.macromates.com/2014/defining-a-path/ [2] https://github.com/mrdougal/textmate2-rubocop/pull/11

[sanssecours]

"Note that while you may already have set the PATH variable for your shell (e.g. bash) it is not inherited in TextMate, as TextMate does not parse your shell setup scripts." [1].

Yes, I know. I assume that PATH inside TextMate usually contains something useful. For example, here is my current configuration of PATH:

SHIMS = "$HOME/.pyenv/shims:$HOME/.rbenv/shims"
PATH  = "$SHIMS:/usr/local/bin:$PATH:$HOME/bin:/opt/local/bin"

The diff also contains /usr/bin/rubocop in requiredCommands, not really sure what that is.

The variable requiredCommands specifies a list of (shell) commands required by the current (TextMate) command. TextMate tries to locate a required command by checking the paths specified via the locations array of the command. If this is not successful, then TextMate takes a look at PATH.

But since OS X 10.11 you don't have write access to /usr/bin. So there's no point in looking for something there which doesn't ship with the OS.

You are right. I changed the path to $HOME/.rbenv/shims/rubocop. That is the location of rubocop on my machine.

No, but the whole editor freezes. You cannot select text, switch tabs, nothing.

The command now runs in the background (see commit e2f94ab).

I like to view the information about the warning/errors in the document, even if there are none.

How would you do that if there are none?

The command just displays the default output of RuboCop (in this case):

If I clone your version of the bundle and run “Analyse current file” I get the following error message

You need my PR [2] 😃.

I forgot to switch to the branch issue7. It works now. Thank you 😘.

Personally I don't think this command is generic and well implement enough to be added upstream.

I hope my recent updates changed your opinion 😁.

[jacob-carlborg]

I hope my recent updates changed your opinion

It looks a lot better now. Although I would still like to see support for Bundler and RVM.

[sanssecours]

It looks a lot better now.

Yay \o/.

Although I would still like to see support for Bundler and RVM.

I added support for Bundler and RVM in the latest commits.

[jacob-carlborg]

LGTM. Please also squash all commits. Could you please write in the commit message how it finds Ruby, RuboCop, in which order the different locations are searched and so on.

[sanssecours]

Please also squash all commits.

Maybe I will do that later, but currently I prefer to keep the commit history. Sorry.

Could you please write in the commit message how it finds Ruby, RuboCop, in which order the different locations are searched and so on.

I added a help document in commit 25e5e3a.

[jacob-carlborg]

Everything looks good to me 👍. Unfortunately I don't have permission to accept the PR.

[sorbits]

@infininight Do you want me to review and merge or can you handle this?

[sanssecours]

Since Allan just added RubyUtils.find_executableto the bundle, I updated this branch with the newest Ruby version of “Reformat Document”. If I should change anything, then please just comment below.

[noniq]

Looks great!

[jacob-carlborg]

All the whitespace related cops in RuboCop have recently been moved to their own group [1], called Layout. That means that only those cops can be invoked using rubocop --only Layout --auto-correct, which is the closest we'll currently get to a Ruby formatter.

[1] https://github.com/bbatsov/rubocop/pull/4278

[sanssecours]

All the whitespace related cops in RuboCop have recently been moved to their own group [1], called Layout. That means that only those cops can be invoked using rubocop --only Layout --auto-correct, which is the closest we'll currently get to a Ruby formatter.

Thank you for the information Jacob. Since I prefer the default behaviour, of fixing as many issues as possible, I added support for an optional environment variable called TM_RUBOCOP_OPTIONS in commit a9d8f86e. You can set this variable to the value --only Layout if you prefer “Reformat Document” only fixing layout specific problems.

[jacob-carlborg]

@sanssecours Well, I think if the command it's called Reformat Document then it should only reformat the document 😉. If you want to have a generic way to invoke RuboCop that's something different.

[sanssecours]

@sanssecours Well, I think if the command it's called Reformat Document then it should only reformat the document 😉.

I understand your reasoning, but still, I would like the command to fix as much problematic code as possible. I renamed the command to “Tidy” in the latest commit. Hope you like that name better.

If you want to have a generic way to invoke RuboCop that's something different.

Yeah, I know. Currently I use Stefan’s bundle for that purpose. I then use “Tidy” to autocorrect most problems reported by the bundle.

[noniq]

Thanks for mentioning my RuboCop bundle 😀 In a way it feels strange that there are two bundles, Ruby and RuboCop, both incorporating RuboCop, even if the use cases are slightly different (tidying vs. linting). Do you think it would make sense to either port the linting part to the Ruby bundle, or the tidying part to the RuboCop bundle? (Note that I’m not sure about this myself, merely thinking out loud …)

[sanssecours]

Do you think it would make sense to either port the linting part to the Ruby bundle, or the tidying part to the RuboCop bundle?

I think both options make sense.

For example, the Python bundle already supports linting via the command “Validate Syntax (PyCheckMate)” bound to ^ + ⇧ + V. The Perl bundle also includes a linting command.

On the other hand, for the sake of modularity, it also makes sense to keep RuboCop specific commands in their own bundle.

[jacob-carlborg]

I renamed the command to “Tidy” in the latest commit

Yeah, that's a bit better.

[noniq]

Just an idea: Are “Reformat” and “Tidy” maybe both useful enough to justify separate commands?

[noniq]

I just realized that the Ruby bundle already has a “Validate Syntax” command (it uses ruby -wc). So we could either replace this command, provide an additional “Validate with RuboCop” command, or keep the RuboCop bundle separate. Hmm.

[sanssecours]

Just an idea: Are “Reformat” and “Tidy” maybe both useful enough to justify separate commands?

I suppose most people would just use one of these commands more or less exclusively. If we really want different behaviour we can still set TM_RUBOCOP_OPTIONS to change the behaviour depending on the current project/file. In general, I think we should not make things more complicated if we do not need to.

I just realized that the Ruby bundle already has a “Validate Syntax” command (it uses ruby -wc). So we could either replace this command, provide an additional “Validate with RuboCop” command, or keep the RuboCop bundle separate.

Since I never use “Validate Syntax” I would vote for just replacing the command. However other people might think quite different. Maybe we could just specify the behaviour of the command via an environment variable, like TM_RUBY_CHECKER or something similar. We could also just use ruby to check the current file if rubocop is currently not available.

[noniq]

I like the idea of using RuboCop if available and falling back to ruby -wc otherwise! This also means that people who don’t care about RuboCop won’t be affected by this change (assuming that they don’t have RuboCop installed at all).

So generally I now prefer the idea of incorporating RuboCop syntax checking into the Ruby bundle. There’s one catch, though: Currently, if you install the RuboCop bundle you also get automatic syntax checking on every save. This might be too intrusive as a default in the Ruby bundle, what do you think?

[jacob-carlborg]

I like the idea of using RuboCop if available and falling back to ruby -wc otherwise! This also means that people who don’t care about RuboCop won’t be affected by this change (assuming that they don’t have RuboCop installed at all).

Since I never use “Validate Syntax” I would vote for just replacing the command. However other people might think quite different. Maybe we could just specify the behaviour of the command via an environment variable, like TM_RUBY_CHECKER or something similar. We could also just use ruby to check the current file if rubocop is currently not available.

The Tidy command and the command to validate the syntax have quite different behavior since the Tidy command will actually change the document (the auto correct).

This might be too intrusive as a default in the Ruby bundle, what do you think?

Yes, that's too intrusive, at least to have enabled by default.

[sanssecours]

So generally I now prefer the idea of incorporating RuboCop syntax checking into the Ruby bundle.

That sounds great, especially since this means I do not have to manually update the RuboCop bundle any more.

There’s one catch, though: Currently, if you install the RuboCop bundle you also get automatic syntax checking on every save. This might be too intrusive as a default in the Ruby bundle, what do you think?

I think that depends mostly on the checking command. I think syntax errors – as reported by ruby -c – are important enough, that it would make sense to report them even by default. RuboCop warnings on the other hand should be disabled by default in my opinion.

The Tidy command and the command to validate the syntax have quite different behavior since the Tidy command will actually change the document (the auto correct).

Actually I was talking about the “Run RuboCop“ checking command provided by Stefan’s Bundle:

. I do not want to replace “Validate Syntax” with “Tidy“. Rather my proposal is, that we extend the command “Validate Syntax“ with the optional possibility to also use rubocop instead of ruby -wc. Sorry that I did not make that clear enough.

[noniq]

Regarding (not) running RuboCop on every save by default:

There’s this concept of enabling / disabling items in the bundle editor (“Enable this item” checkbox right at the top). Maybe we could simply disable the command for linting on save (it’s a separate command anyway), and tell people to manually enable it if they want to use this featue?

[jacob-carlborg]

Maybe we could simply disable the command for linting on save (it’s a separate command anyway), and tell people to manually enable it if they want to use this featue?

Sounds good to me.

[noniq]

Fine! I’ll create a pull request as soon as this one has been merged. 🚢

[noniq]

So what’s the state here? Do we have to ping Michael or Allan to get this merged? 🤔

[jacob-carlborg]

So what’s the state here? Do we have to ping Michael or Allan to get this merged?

I would guess so.

[noniq]

Ping @infininight @sorbits :)

[infininight]

Sorry, been meaning to getting around to looking at this. I just got done with a once over of the Lua bundle and am switching to the ruby bundle now.

So currently there are three commands being talked about here:

Validation: Code not touched, errors flagged Reformat: Code layout changed, whitespace(?) only, no real code changes Cleanup: Code changed and reformatted based on RuboCop

The command added here being the third. Currently we only really have names in TextMate for the first two items Validate Syntax and Reformat Document respectively. A few commands do use the name "Tidy" but I'm not sure how I feel about the usage here. To mean the connotation of 'tidy' brings to mind the HTML/XML command, I don't know how many people are really familiar with the term. Even taking that into account though all tidy does is the second level of changing not real code changes.

Don't really have a great suggestion here, just don't like Tidy much. Was the final decision to only have a Validation and Tidy command and let the TM_RUBOCOP_OPTIONS option serve as the second option?

As for the 'on-save' command, yeah I don't like having those in installable bundles, but it's perfectly fine to add it disabled and let users enable it if they so desire.

[sorbits]

Currently we only really have names in TextMate for the first two items Validate Syntax and Reformat Document

In the C bundle we have Reformat Document which relies on clang-format, it could add and remove braces, etc., so fairly invasive.

In the Go bundle we have a command that calls gofmt, interestingly the command is called Fmt — should probably be renamed :)

I think everybody knows about tidy, but given the gofmt and clang-format, I think Reformat Document is more appropriate for programming languages, and perhaps HTML/CSS should even be changed to Reformat Document (tidy).

[jacob-carlborg]

I think everybody knows about tidy, but given the gofmt and clang-format, I think Reformat Document is more appropriate for programming languages

I think that RuboCop is a bit more invasive that clang-format. For example, it will change the following code:

[1, 2, 3].map { |e|
  p "1"
}

To:

[1, 2, 3].map do |_e|
  p '1'
end

Oh, and BTW, Clang does actually have a tidy tool called clang-tidy [1]

[1] http://clang.llvm.org/extra/clang-tidy/

[sanssecours]

Was the final decision to only have a Validation and Tidy command and let the TM_RUBOCOP_OPTIONS option serve as the second option?

I do not think that there was any conclusion about that. My opinion is still, that it does not make that much sense to add two very similar commands (“Reformat Layout (Only)”, “Reformat Document”) as I argued before:

I suppose most people would just use one of these commands more or less exclusively. If we really want different behaviour we can still set TM_RUBOCOP_OPTIONS to change the behaviour depending on the current project/file. In general, I think we should not make things more complicated if we do not need to.

.

---

Since both Michael:

Don't really have a great suggestion here, just don't like Tidy much.

and Allan:

…I think Reformat Document is more appropriate for programming languages…

are not happy with the name “Tidy” I just reverted the last commit. The command is now called “Reformat Document” once again.

---

In the C bundle we have Reformat Document which relies on clang-format, it could add and remove braces, etc., so fairly invasive.

As far as I can tell the current version of clang-format can not add or remove braces. However, there is a Clang tool that is able to make such changes called Clang-Tidy 😊, as Jacob already mentioned.

I think that RuboCop is a bit more invasive than clang-format.

I think so too, but since RuboCop does not change the semantics of a program I still think it is okay to call the command “Reformat Document”. At least the description of “reformat” from the “New Oxford American Dictionary”:

verb (reformats, reformatting, reformatted) [with object] chiefly Computing give a new format to; revise or represent in another format.

applies (according to my opinion of course :o)).

[infininight]

Pulled as 11ad1b9bb7dded9c289727244b0d549a033c36bb...368ea39081b432892c418e00a0682c1508321d71, with the addition of bd70e6aad45e0890cf257fa9a6663dbe1ffecdc1 for a more info link, otherwise unchanged. Thanks!