Remove forceful exit #6452
Remove forceful exit #6452
Conversation
If passed nil, `warn` seems to do nothing. I guess the intention here was to print a new line.
Even on a big project with a lot of files and cops, graceful exit is inmediate, so I don't see the need for this feature. As a matter of fact, I tried to exercise the feature, but I'm enable to run Ctrl-C a second time before rubocop exits gracefully, so I'm not sure there's a potentially expensive cleanup operation going on at all here.
| @@ -72,6 +80,8 @@ def inspect_files(files) | |||
| end | |||
|
|
|||
| def each_inspected_file(files) | |||
| trap_interrupt | |||
There was a problem hiding this comment.
Does this need to be done for each file?
There was a problem hiding this comment.
I don't think so, we should install the interrupt handling just once before analysis starts. In any case, the handler is completely going away in #6461.
|
I think @yujinakayama wrote the original code, so it'd be nice to hear what were his reason to implement the two-stage interrupt. I think this was a common approach taken by several tools, and we just borrowed it, but my memory is a bit fuzzy. |
|
Yeah, the original approach seems taken from |
|
I gave this some more though, and I managed to figure out a situation where one would not get an immediate exit when pressing Ctrl-C with this patch. If Ctrl-C is pressed during analysis of a huge file (> 10000 lines), then under the current patch one would have to wait until the analysis of that file finishes and that's not immediate. I'll give it some more thought to see if I can keep the current behavior while improving the stuff I wanted to improve. |
|
For now I'm closing this in favor of #6537! |
Up to now, interrupt handling was implemented in two phases. If user would press Ctrl-C, rubocop would start exiting gracefully with the following message "Exiting... Interrupt again to exit immediately.", and if Ctrl-C was pressed again,
rubocopwould exit without any further cleanup.However, I don't think this behavior is necessary. I tried to interrupt rubocop running without or without cache, in big or small projects, and I always get a graceful exit without even having any time to trigger the forceful exit path. So, I think we can simplify things and get rid of it.
This is actually some preparation to fix the actual end user problems I wanted to fix in #5808, but does not yet fix them. I still think this is a code simplification and an improvement in the end user experience.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.