-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add hyperlinks of stack traces in Laravel logs #147
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package com.intellij.ideolog.filters | ||
|
||
import com.intellij.execution.filters.Filter | ||
import com.intellij.execution.filters.HyperlinkInfo | ||
import com.intellij.execution.filters.LazyFileHyperlinkInfo | ||
import com.intellij.openapi.project.Project | ||
import com.intellij.openapi.util.text.StringUtil | ||
import com.intellij.openapi.vfs.LocalFileSystem | ||
import java.util.regex.Matcher | ||
import java.util.regex.Pattern | ||
|
||
class LaravelStackTraceFileFilter( | ||
private val project: Project, | ||
private val localFileSystem: LocalFileSystem | ||
) : Filter { | ||
companion object { | ||
private val LINUX_MACOS_FILE_PATTERN = | ||
Pattern.compile("\\B/[-A-Za-z0-9+$&@#/%?=~_|!:,.;]*[-A-Za-z0-9+$&@#/%=~_|]\\(\\d+\\)") | ||
private val WINDOWS_FILE_PATTERN = | ||
Pattern.compile("\\b[A-Z]:\\\\[-A-Za-z0-9+$&@#\\\\%=~_!:,.;]*[-A-Za-z0-9+$&@#/%=~_|]\\(\\d+\\)") | ||
} | ||
|
||
override fun applyFilter(line: String, entireLength: Int): Filter.Result? { | ||
val textStartOffset = entireLength - line.length | ||
val items = collectItems(textStartOffset, LINUX_MACOS_FILE_PATTERN.matcher(line)) + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance: we probably should add some kind of fast check that line is not Url to avoid checking regexps |
||
collectItems(textStartOffset, WINDOWS_FILE_PATTERN.matcher(line)) | ||
|
||
return when (items.size) { | ||
0 -> null | ||
1 -> Filter.Result(items[0].highlightStartOffset, items[0].highlightEndOffset, items[0].hyperlinkInfo) | ||
else -> Filter.Result(items) | ||
} | ||
} | ||
|
||
private fun collectItems(textStartOffset: Int, matcher: Matcher): List<Filter.ResultItem> { | ||
val resultItems = mutableListOf<Filter.ResultItem>() | ||
while (matcher.find()) { | ||
resultItems.add(Filter.ResultItem( | ||
textStartOffset + matcher.start(), | ||
textStartOffset + matcher.end(), | ||
buildFileHyperlinkInfo(matcher.group())) | ||
) | ||
} | ||
return resultItems | ||
} | ||
|
||
private fun buildFileHyperlinkInfo(fileUri: String): HyperlinkInfo? { | ||
var documentLine = 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's always a better idea to declare variables as close to their first usage as possible. Also, let's avoid using
|
||
val filePathEndIndex = fileUri.lastIndexOf('(') | ||
pestretsov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
val filePath = fileUri.substring(0, filePathEndIndex) | ||
localFileSystem.findFileByPath(filePath) ?: return null | ||
|
||
val possibleDocumentLine = StringUtil.parseInt(fileUri.substring(filePathEndIndex + 1, fileUri.lastIndex), Int.MIN_VALUE) | ||
if (possibleDocumentLine != Int.MIN_VALUE) { | ||
documentLine = possibleDocumentLine - 1 | ||
} | ||
return LinedFileHyperlinkInfo(project, filePath, documentLine) | ||
} | ||
|
||
class LinedFileHyperlinkInfo( | ||
project: Project, | ||
val filePath: String, | ||
val documentLine: Int, | ||
) : LazyFileHyperlinkInfo(project, filePath, documentLine, 0, false) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.intellij.ideolog.filters | ||
|
||
import com.intellij.execution.filters.ConsoleFilterProvider | ||
import com.intellij.execution.filters.Filter | ||
import com.intellij.openapi.project.Project | ||
import com.intellij.openapi.vfs.LocalFileSystem | ||
|
||
class LaravelStackTraceFileFilterProvider : ConsoleFilterProvider { | ||
override fun getDefaultFilters(project: Project): Array<Filter> { | ||
return arrayOf(LaravelStackTraceFileFilter(project, LocalFileSystem.getInstance())) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
package com.intellij.ideolog.filters | ||
|
||
import com.intellij.execution.filters.Filter | ||
import com.intellij.openapi.vfs.LocalFileSystem | ||
import com.intellij.testFramework.RunsInEdt | ||
import com.intellij.testFramework.fixtures.BasePlatformTestCase | ||
import junit.framework.TestCase | ||
import org.junit.rules.TemporaryFolder | ||
|
||
@RunsInEdt | ||
internal class LaravelStackTraceFileFilterTests : BasePlatformTestCase() { | ||
|
||
private lateinit var filter: LaravelStackTraceFileFilter | ||
private val tmpFolder: TemporaryFolder = TemporaryFolder() | ||
|
||
override fun setUp() { | ||
super.setUp() | ||
tmpFolder.create() | ||
filter = LaravelStackTraceFileFilter(project, LocalFileSystem.getInstance()) | ||
} | ||
|
||
override fun tearDown() { | ||
super.tearDown() | ||
tmpFolder.delete() | ||
} | ||
|
||
fun `test no file hyperlink`() { | ||
assertNoFileHyperlink("") | ||
assertNoFileHyperlink("No file hyperlink") | ||
assertNoFileHyperlink("""/Users\me/Application.php""") | ||
assertNoFileHyperlink("""C:\Users\me/Application.php""") | ||
} | ||
|
||
fun `test no Laravel logs file hyperlink`() { | ||
assertNoFileHyperlink("/Users/me/Application.php:35") | ||
assertNoFileHyperlink("file:///Users/me/Application.php:35") | ||
assertNoFileHyperlink("""C:\Users\me\Application.php:35""") | ||
} | ||
|
||
fun `test single Laravel logs file hyperlink`() { | ||
assertFileHyperlink("/Users/me/Application.php(35)", 0, 29, "/Users/me/Application.php", 35, false) | ||
assertFileHyperlink("""C:\Users\me\Application.php(34)""", 0, 31, """C:\Users\me\Application.php""", 34, false) | ||
} | ||
|
||
fun `test multiple Laravel logs file hyperlinks`() { | ||
assertFileHyperlinks( | ||
applyFilter("{ #stacktrace: /Users/me/Application.php(35) /Users/me/Kernel.php(42) }"), | ||
listOf( | ||
FileLinkInfo(15, 44, "/Users/me/Application.php", 35, false), | ||
FileLinkInfo(45, 69, "/Users/me/Kernel.php", 42, false) | ||
) | ||
) | ||
} | ||
|
||
fun `test apply Filter to existing Laravel file path on linux or mac`() { | ||
val existingFile = tmpFolder.newFile("Application.php") | ||
val filePathLength = existingFile.absolutePath.length | ||
assertFileHyperlink("#0 ${existingFile.absolutePath}(2)", 3, 6 + filePathLength, existingFile.absolutePath, 2, true) | ||
} | ||
|
||
fun `test apply Filter to multiple existing Laravel files path on linux or mac`() { | ||
val firstExistingFile = tmpFolder.newFile("Application.php") | ||
val firstFilePathLength = firstExistingFile.absolutePath.length | ||
val secondExistingFile = tmpFolder.newFile("Kernel.php") | ||
val secondFilePathLength = secondExistingFile.absolutePath.length | ||
assertFileHyperlinks( | ||
applyFilter("#0 ${firstExistingFile.absolutePath}(2), ${secondExistingFile.absolutePath}(4)"), | ||
listOf( | ||
FileLinkInfo(3, 6 + firstFilePathLength, firstExistingFile.absolutePath, 2, true), | ||
FileLinkInfo(8 + firstFilePathLength, 11 + firstFilePathLength + secondFilePathLength, secondExistingFile.absolutePath, 4, true) | ||
) | ||
) | ||
} | ||
|
||
private fun applyFilter(line: String) = filter.applyFilter(line, line.length) | ||
|
||
private fun assertNoFileHyperlink(text: String) { | ||
assertNull(applyFilter(text)) | ||
} | ||
|
||
private fun assertFileHyperlink( | ||
text: String, | ||
highlightStartOffset: Int, | ||
highlightEndOffset: Int, | ||
filePath: String, | ||
documentLine: Int, | ||
isFileExists: Boolean | ||
) { | ||
assertFileHyperlinks( | ||
applyFilter(text), | ||
listOf(FileLinkInfo(highlightStartOffset, highlightEndOffset, filePath, documentLine, isFileExists)) | ||
) | ||
} | ||
|
||
private fun assertFileHyperlinks(result: Filter.Result?, infos: List<FileLinkInfo>) { | ||
assertNotNull(result) | ||
result?.let { | ||
val items = result.resultItems | ||
assertEquals(infos.size, items.size) | ||
infos.indices.forEach { assertHyperlink(items[it], infos[it]) } | ||
} | ||
} | ||
|
||
private fun assertHyperlink(actualItem: Filter.ResultItem, expectedFileLinkInfo: FileLinkInfo) { | ||
assertEquals(expectedFileLinkInfo.highlightStartOffset, actualItem.highlightStartOffset) | ||
assertEquals(expectedFileLinkInfo.highlightEndOffset, actualItem.highlightEndOffset) | ||
if (expectedFileLinkInfo.isFileExists) { | ||
assertInstanceOf(actualItem.hyperlinkInfo, LaravelStackTraceFileFilter.LinedFileHyperlinkInfo::class.java) | ||
assertFileLink(expectedFileLinkInfo, actualItem.hyperlinkInfo as LaravelStackTraceFileFilter.LinedFileHyperlinkInfo) | ||
} else { | ||
TestCase.assertNull(actualItem.hyperlinkInfo) | ||
} | ||
} | ||
|
||
private fun assertFileLink(expected: FileLinkInfo, actual: LaravelStackTraceFileFilter.LinedFileHyperlinkInfo) { | ||
assertEquals(expected.filePath, actual.filePath) | ||
assertEquals(expected.line, actual.documentLine + 1) | ||
} | ||
|
||
data class FileLinkInfo( | ||
val highlightStartOffset: Int, | ||
val highlightEndOffset: Int, | ||
val filePath: String, | ||
val line: Int, | ||
val isFileExists: Boolean | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
items
andcollectItems
are kinda too generic names, right?