Skip to content

Commit

Permalink
KT-10974 - code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@gcx11
gcx11 committed May 1, 2020
1 parent 093be08 commit 92cb6c6
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 72 deletions.
11 changes: 5 additions & 6 deletions idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinPackageEntry.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.jetbrains.kotlin.idea.core.formatter

import com.intellij.openapi.application.ApplicationBundle
import org.jetbrains.kotlin.resolve.ImportPath

class KotlinPackageEntry(
Expand All @@ -15,14 +16,15 @@ class KotlinPackageEntry(

companion object {
@JvmField
val ALL_OTHER_IMPORTS_ENTRY = KotlinPackageEntry("<all other imports>", withSubpackages = true)
val ALL_OTHER_IMPORTS_ENTRY =
KotlinPackageEntry(ApplicationBundle.message("listbox.import.all.other.imports"), withSubpackages = true)

@JvmField
val ALL_OTHER_ALIAS_IMPORTS_ENTRY = KotlinPackageEntry("<all other alias imports>", withSubpackages = true)
}

fun matchesPackageName(otherPackageName: String): Boolean {
if (this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY) return true
if (isSpecial) return true

if (otherPackageName.startsWith(packageName)) {
if (otherPackageName.length == packageName.length) return true
Expand All @@ -47,10 +49,7 @@ class KotlinPackageEntry(
return entry.packageName.count { it == '.' } < packageName.count { it == '.' }
}

val isSpecial: Boolean
get() {
return (this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY)
}
val isSpecial: Boolean get() = this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY

override fun toString(): String {
return packageName
Expand Down
29 changes: 4 additions & 25 deletions idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinPackageEntryTable.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,17 @@ import com.intellij.openapi.util.InvalidDataException
import com.intellij.openapi.util.JDOMExternalizable
import org.jdom.Element

class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
class KotlinPackageEntryTable : JDOMExternalizable, Cloneable {
private val entries = mutableListOf<KotlinPackageEntry>()

override fun equals(other: Any?): Boolean {
if (other !is KotlinPackageEntryTable) return false
if (other.entries.size != entries.size) return false

return entries.zip(other.entries).any { (entry, otherEntry) -> entry != otherEntry }
}
val entryCount: Int get() = entries.size

public override fun clone(): KotlinPackageEntryTable {
val clone = KotlinPackageEntryTable()
clone.copyFrom(this)
return clone
}

override fun hashCode(): Int {
return entries.firstOrNull()?.hashCode() ?: 0
}

fun copyFrom(packageTable: KotlinPackageEntryTable) {
entries.clear()
entries.addAll(packageTable.entries)
Expand All @@ -50,24 +41,12 @@ class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
return entries[index]
}

fun getEntryCount(): Int {
return entries.size
}

fun setEntryAt(entry: KotlinPackageEntry, index: Int) {
entries[index] = entry
}

operator fun contains(packageName: String): Boolean {
for (entry in entries) {
if (packageName.startsWith(entry.packageName)) {
if (packageName.length == entry.packageName.length) return true
if (entry.withSubpackages) {
if (packageName[entry.packageName.length] == '.') return true
}
}
}
return false
return entries.any { !it.isSpecial && it.matchesPackageName(packageName) }
}

fun removeEmptyPackages() {
Expand Down Expand Up @@ -102,7 +81,7 @@ class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
for (entry in entries) {
val element = Element("package")
parentNode.addContent(element)
val name = if (entry == KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY || entry == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY) "" else entry.packageName
val name = if (entry.isSpecial) "" else entry.packageName
val alias = (entry == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)

element.setAttribute("name", name)
Expand Down
61 changes: 31 additions & 30 deletions idea/src/org/jetbrains/kotlin/idea/formatter/BaseKotlinImportLayoutPanel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import javax.swing.JTable
import javax.swing.ListSelectionModel
import javax.swing.table.AbstractTableModel

open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
open class BaseKotlinImportLayoutPanel(title: String) : JPanel(BorderLayout()) {
val packageTable = KotlinPackageEntryTable()
val layoutTable = createTableForPackageEntries(packageTable)

Expand All @@ -41,7 +41,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
protected fun addPackage() {
var row = layoutTable.selectedRow + 1
if (row < 0) {
row = packageTable.getEntryCount()
row = packageTable.entryCount
}
val entry = KotlinPackageEntry("", true)
packageTable.insertEntryAt(entry, row)
Expand All @@ -53,17 +53,15 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
if (row < 0) return

val entry = packageTable.getEntryAt(row)
if (entry == KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY || entry == KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY) {
return
}
if (entry.isSpecial) return

TableUtil.stopEditing(layoutTable)
packageTable.removeEntryAt(row)

val model = layoutTable.model as AbstractTableModel
model.fireTableRowsDeleted(row, row)

if (row >= packageTable.getEntryCount()) {
if (row >= packageTable.entryCount) {
row--
}

Expand All @@ -89,7 +87,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {

protected fun movePackageDown() {
val row = layoutTable.selectedRow
if (row >= packageTable.getEntryCount() - 1) return
if (row >= packageTable.entryCount - 1) return

TableUtil.stopEditing(layoutTable)
val entry = packageTable.getEntryAt(row)
Expand All @@ -109,7 +107,8 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
TableUtil.editCellAt(layoutTable, row, 0)
val editorComp = layoutTable.editorComponent
if (editorComp != null) {
IdeFocusManager.getGlobalInstance().doWhenFocusSettlesDown { IdeFocusManager.getGlobalInstance().requestFocus(editorComp, true) }
IdeFocusManager.getGlobalInstance()
.doWhenFocusSettlesDown { IdeFocusManager.getGlobalInstance().requestFocus(editorComp, true) }
}
}

Expand Down Expand Up @@ -156,7 +155,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
}

private fun fixColumnWidthToHeader(columnIndex: Int) {
with (layoutTable) {
with(layoutTable) {
val column = columnModel.getColumn(columnIndex)
val width = 15 + tableHeader.getFontMetrics(tableHeader.font).stringWidth(getColumnName(columnIndex))

Expand All @@ -166,14 +165,14 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
}
}

class KotlinStarImportLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.packages.to.use.import.with")) {
class KotlinStarImportLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.packages.to.use.import.with")) {
init {
val importLayoutPanel = ToolbarDecorator.createDecorator(layoutTable)
.setAddAction { addPackage() }
.setRemoveAction { removePackage() }
.setButtonComparator(
"Add",
"Remove"
ApplicationBundle.message("button.add.package"),
ApplicationBundle.message("button.remove"),
).setPreferredSize(Dimension(-1, 100))
.createPanel()

Expand All @@ -182,15 +181,15 @@ class KotlinStarImportLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundle
}
}

class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.import.layout")) {
class KotlinImportOrderLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.import.layout")) {
private val cbImportAliasesSeparately = JBCheckBox("Import aliases separately")

init {
add(cbImportAliasesSeparately, BorderLayout.NORTH)

val importLayoutPanel = ToolbarDecorator.createDecorator(layoutTable)
.addExtraAction(
object: DumbAwareActionButton(ApplicationBundle.message("button.add.package"), IconUtil.getAddPackageIcon()) {
object : DumbAwareActionButton(ApplicationBundle.message("button.add.package"), IconUtil.getAddPackageIcon()) {
override fun actionPerformed(event: AnActionEvent) {
addPackage()
}
Expand All @@ -205,14 +204,14 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
.setMoveDownAction { movePackageDown() }
.setRemoveActionUpdater {
val selectedRow = layoutTable.selectedRow
val entry = if (selectedRow in 0 until packageTable.getEntryCount()) packageTable.getEntryAt(selectedRow) else null
val entry = if (selectedRow in 0 until packageTable.entryCount) packageTable.getEntryAt(selectedRow) else null

entry != null && entry != KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY && entry != KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY
entry?.isSpecial == false
}.setButtonComparator(
ApplicationBundle.message("button.add.package"),
"Remove",
"Up",
"Down"
ApplicationBundle.message("button.remove"),
ApplicationBundle.message("button.move.up"),
ApplicationBundle.message("button.move.down")
).setPreferredSize(Dimension(-1, 100))
.createPanel()

Expand All @@ -221,15 +220,15 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl

cbImportAliasesSeparately.addItemListener { _ ->
if (areImportAliasesEnabled()) {
if (packageTable.getEntries().none { it == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY }) {
if (KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY !in packageTable.getEntries()) {
packageTable.addEntry(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)
val row = packageTable.getEntryCount() - 1
val row = packageTable.entryCount - 1
val model = layoutTable.model as AbstractTableModel
model.fireTableRowsInserted(row, row)
layoutTable.setRowSelectionInterval(row, row)
}
} else {
val entryIndex = packageTable.getEntries().indexOfFirst { it == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY }
val entryIndex = packageTable.getEntries().indexOf(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)

if (entryIndex != -1) {
val currentIndex = layoutTable.selectedRow
Expand All @@ -248,7 +247,7 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
}

fun recomputeAliasesCheckbox() {
cbImportAliasesSeparately.isSelected = packageTable.getEntries().any { it == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY }
cbImportAliasesSeparately.isSelected = KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY in packageTable.getEntries()
}

private fun areImportAliasesEnabled(): Boolean {
Expand All @@ -258,23 +257,25 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl

fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable {
val names = arrayOf(ApplicationBundle.message("listbox.import.package"), ApplicationBundle.message("listbox.import.with.subpackages"))
val packageNameColumnIndex = 0
val withSubpackagesColumnIndex = 1

val dataModel = object : AbstractTableModel() {
override fun getColumnCount(): Int {
return names.size
}

override fun getRowCount(): Int {
return packageTable.getEntryCount()
return packageTable.entryCount
}

override fun getValueAt(rowIndex: Int, columnIndex: Int): Any? {
val entry = packageTable.getEntryAt(rowIndex)
if (!isCellEditable(rowIndex, columnIndex)) return null

return when (columnIndex) {
0 -> entry.packageName
1 -> entry.withSubpackages
packageNameColumnIndex -> entry.packageName
withSubpackagesColumnIndex -> entry.withSubpackages
else -> throw IllegalArgumentException(columnIndex.toString())
}
}
Expand All @@ -290,8 +291,8 @@ fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable

override fun getColumnClass(columnIndex: Int): Class<*> {
return when (columnIndex) {
0 -> String::class.java
1 -> Boolean::class.javaObjectType
packageNameColumnIndex -> String::class.java
withSubpackagesColumnIndex -> Boolean::class.javaObjectType
else -> throw IllegalArgumentException(columnIndex.toString())
}
}
Expand All @@ -300,8 +301,8 @@ fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable
val entry = packageTable.getEntryAt(rowIndex)

val newEntry = when (columnIndex) {
0 -> KotlinPackageEntry((value as String).trim(), entry.withSubpackages)
1 -> KotlinPackageEntry(entry.packageName, value.toString().toBoolean())
packageNameColumnIndex -> KotlinPackageEntry((value as String).trim(), entry.withSubpackages)
withSubpackagesColumnIndex -> KotlinPackageEntry(entry.packageName, value.toString().toBoolean())
else -> throw IllegalArgumentException(columnIndex.toString())
}

Expand Down
4 changes: 2 additions & 2 deletions idea/src/org/jetbrains/kotlin/idea/formatter/ImportSettingsPanel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,11 @@ class ImportSettingsPanel(private val commonSettings: CodeStyleSettings) : JPane
}

private fun isModified(list: KotlinPackageEntryTable, table: KotlinPackageEntryTable): Boolean {
if (list.getEntryCount() != table.getEntryCount()) {
if (list.entryCount != table.entryCount) {
return true
}

for (i in 0 until list.getEntryCount()) {
for (i in 0 until list.entryCount) {
val entry1 = list.getEntryAt(i)
val entry2 = table.getEntryAt(i)
if (entry1 != entry2) {
Expand Down
14 changes: 5 additions & 9 deletions idea/src/org/jetbrains/kotlin/idea/util/ImportPathComparator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@ import java.util.Comparator
class ImportPathComparator(
private val packageTable: KotlinPackageEntryTable
) : Comparator<ImportPath> {
override fun compare(import1: ImportPath, import2: ImportPath): Int {
val index1 = bestMatchIndex(import1)
val index2 = bestMatchIndex(import2)
private val comparator = compareBy<ImportPath>(
{ import -> bestMatchIndex(import) },
{ import -> import.toString() }
)

return when {
index1 == index2 -> import1.toString().compareTo(import2.toString())
index1 < index2 -> -1
else -> +1
}
}
override fun compare(import1: ImportPath, import2: ImportPath): Int = comparator.compare(import1, import2)

private fun bestMatchIndex(path: ImportPath): Int {
var bestIndex: Int? = null
Expand Down

0 comments on commit 92cb6c6

Please sign in to comment.