Feature/ide url handler #265
Conversation
src/Application/Console/Analyser.php
Outdated
| @@ -52,6 +52,10 @@ public function analyse( | |||
| $insightCollection = $this->insightCollectionFactory | |||
| ->get($metrics, $config, $dir, $consoleOutput); | |||
|
|
|||
| if (method_exists($formatter, 'injectFileLinkFormatter')) { | |||
There was a problem hiding this comment.
Not sure if I like this approach. I feel like it doesn't belong here. Could we just give the format method the config file or the config file in the constructor of the formatter?
There was a problem hiding this comment.
Yeah, I'm not convinced either.
I initially added it in format function, but it's not a prerequisite for each formatter, that's why I think adding it in the constructor it's not a good solution...
Maybe by create an Interface RequireFileLinkFormatter ? And adding it if formatter is instance of this interface.
WDYT @olivernybroe ?
There was a problem hiding this comment.
Hmm, still seems kinda weird as that is only needed for that formatter.
What if we in the constructor of the Console formatter just extract the parameter ide from the inputInteface? This way all the logic is in the Console formatter, but no extra parameter is needed.
There was a problem hiding this comment.
In fact, it's used by the Console formatter, but we can imagine use it also in an HTML Formatter. That's why I would prefer keeping the ide resolving outside the Console Formatter.
Maybe the easier would be to create an Config class, that will be instancied when the app boots, and dispatch it through the container ? Then each class that need a part of config can get it from container ?
There was a problem hiding this comment.
I like that idea. That would work great for when we add more features.
Another idea could be to use a trait, but I'll let you choose, I like both ideas.
| @@ -0,0 +1,71 @@ | |||
| # IDE Integration | |||
There was a problem hiding this comment.
I would remove this, and add only:
# IDE Integration
In your `phpinsights.php` file, add the conf...
## Supported IDE
...
## Unsupported IDE
...
There was a problem hiding this comment.
Do you mean without the prerequisite & troubleshouting part ?
Should i display them as tip or warning instead ?
Jibbarth
force-pushed
the
feature/ide-url-handler
branch
3 times, most recently
from
123b558 to
8903d6d
Compare
|
@Jibbarth When the config file PR is merged in, are you then going to update this PR also? |
|
@olivernybroe Sure 😊 |
Jibbarth
force-pushed
the
feature/ide-url-handler
branch
from
8903d6d to
6688396
Compare
Thanks to symfony/console 4.3, we can add hyperlink easily in console.
This PR add a
ideconfig key, (fillable by theses differents editor:phpstorm, sublime, textmate, macvim, emacs, atom and vscode) and generate hyperlinks in terminal to file that have issues.