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

Fix hyperlink for elements of the type 'KtFile' #3386

Merged

Conversation

biazmoreira
Copy link
Contributor

@biazmoreira biazmoreira commented Jan 18, 2021

Issue: #2340

Description:

IntelliJ (and consequently Android Studio), considers the following implementation when enabling hyperlink in its terminal emulator: AbstractFileHyperlinkFilter

I noticed that there were two issues happening related to hyperlinks not being enabled in the output of detect:

  1. Files outside the considered project scope by IntelliJ (/build, etc) were not hyperlinking because IntelliJ uses the Filter above, and consequently, the file must be inside the considered project scope by IntelliJ in order to be Highlighted.
  2. And output with two file paths in the same line -- this usually happens for elements of type KtFile, classes and functions seem to hyperlink just fine. I didn’t find a reason as to why this happens, but I think it’s out of the scope of Detekt to fix it -- other tools have the same problem.

Example of the point 2 before the changes I applied in this PR:
Screenshot 2021-01-17 at 21 42 43


Fix:
A fix for point 2 is to simplify the Entity name in the case of Files by getting the file name without the absolute path since the path is also written along with the output.

Regarding other terminal emulators, it really depends on the terminal emulator you're using to specify the way of hyperlinking: https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda.
This only tackles the IntelliJ Terminal Emulator.

@biazmoreira biazmoreira force-pushed the biancamoreira/fix-hyperlink-file-elements branch from 337c48b to a67ba86 Compare January 18, 2021 11:19
@codecov
Copy link

codecov bot commented Jan 18, 2021

Codecov Report

Merging #3386 (25e9da2) into master (25edc90) will increase coverage by 0.01%.
The diff coverage is 87.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3386      +/-   ##
============================================
+ Coverage     80.45%   80.46%   +0.01%     
- Complexity     2737     2739       +2     
============================================
  Files           447      448       +1     
  Lines          8287     8282       -5     
  Branches       1573     1573              
============================================
- Hits           6667     6664       -3     
+ Misses          772      771       -1     
+ Partials        848      847       -1     
Impacted Files Coverage Δ Complexity Δ
.../main/kotlin/io/gitlab/arturbosch/detekt/Detekt.kt 27.95% <ø> (ø) 19.00 <0.00> (ø)
...gitlab/arturbosch/detekt/internal/DetektAndroid.kt 71.87% <ø> (ø) 12.00 <0.00> (ø)
.../io/gitlab/arturbosch/detekt/internal/DetektJvm.kt 59.25% <ø> (ø) 5.00 <0.00> (ø)
...lin/io/github/detekt/tooling/api/spec/RulesSpec.kt 100.00% <ø> (ø) 0.00 <0.00> (ø)
...otlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt 81.08% <50.00%> (ø) 9.00 <0.00> (ø)
...itlab/arturbosch/detekt/api/internal/Signatures.kt 74.46% <66.66%> (+1.13%) 0.00 <0.00> (ø)
...o/gitlab/arturbosch/detekt/internal/DetektPlain.kt 87.50% <87.50%> (ø) 5.00 <5.00> (?)
.../kotlin/io/gitlab/arturbosch/detekt/cli/CliArgs.kt 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
...ain/kotlin/io/gitlab/arturbosch/detekt/cli/Spec.kt 80.43% <100.00%> (ø) 0.00 <0.00> (ø)
...ab/arturbosch/detekt/core/config/AllRulesConfig.kt 44.44% <100.00%> (ø) 4.00 <2.00> (?)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25edc90...89bba47. Read the comment docs.

@biazmoreira biazmoreira force-pushed the biancamoreira/fix-hyperlink-file-elements branch 3 times, most recently from e31e758 to 88c7eac Compare January 18, 2021 11:58
@biazmoreira biazmoreira force-pushed the biancamoreira/fix-hyperlink-file-elements branch from 88c7eac to 9b5bd96 Compare January 18, 2021 12:05
Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed investigation! This PR looks very good! Finally, this issue gets closed. I like it.

biazmoreira and others added 2 commits January 18, 2021 20:42
…itySpec.kt

Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>
Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

Thanks! Really good investigation!

This was referenced Mar 11, 2021
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.

4 participants