Review - Do Not Merge!

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/Activator.scala

@@ -12,7 +12,7 @@ import scaps.eclipse.core.services.ScapsService
 /**
  * The activator class controls the plug-in life cycle
  */
-
+// Wird dieser gebraucht? Falls keine Referenz darauf -> löschen.
 class Activator extends AbstractUIPlugin {
   /*
    * (non-Javadoc)

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/core/adapters/ScapsAdapter.scala

@@ -30,7 +30,7 @@ class ScapsAdapter(indexDir: String) extends StrictLogging {
 
   private def searchEngine = {
     val conf = Settings.fromApplicationConf.modIndex { index => index.copy(indexDir = indexDir) }
-    SearchEngine(conf).get
+    SearchEngine(conf).get // Error Handling?
   }
 
   def indexProject(classPath: Seq[String], projectSourceUnits: Seq[ICompilationUnit]): Unit = projectSourceUnits match {
@@ -55,11 +55,12 @@ class ScapsAdapter(indexDir: String) extends StrictLogging {
   def indexFinalize = searchEngine.finalizeIndex.get
 
   def search(searchQuery: String): Seq[Result[ValueDef]] = {
-    searchEngine.search(searchQuery, Set()).get.getOrElse(List())
+    searchEngine.search(searchQuery, Set.empty).get.getOrElse(Seq.empty) // Könnte man auch so schreiben, aber eure Variante ist auch ok.
   }
 
   private def indexDefinitions(extractionStream: Stream[ExtractionError \/ Definition]): Unit = {
 
+    // Lohnt sich diese Methode?
     def handleExtractionErrors(extractionStream: Stream[ExtractionError \/ Definition]): Stream[Definition] =
       ExtractionError.logErrors(extractionStream, logger.info(_))
 

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/core/services/ScapsIndexService.scala

@@ -44,11 +44,12 @@ class ScapsIndexService(private val scapsAdapter: ScapsAdapter) extends StrictLo
       resolvedClassPath.map(_.getPath.toString).toList
     }
 
+    // Sehr lange Zeile..
     def returnElements(classPath: Option[List[String]], projectSourceFragmentRoots: Option[List[IPackageFragmentRoot]], librarySourceRootFiles: Option[List[File]]): (List[String], List[IPackageFragmentRoot], List[File]) = {
-      (classPath.getOrElse(List[String]()), projectSourceFragmentRoots.getOrElse(List[IPackageFragmentRoot]()), librarySourceRootFiles.getOrElse(List[File]()))
+      (classPath.getOrElse(List.empty), projectSourceFragmentRoots.getOrElse(List.empty), librarySourceRootFiles.getOrElse(List.empty))
     }
 
-    val collectedElements = scapsWorkingSet.getElements.toList.map(_ match {
+    val (classPaths, projectSourceFragmentRoots, librarySourceRootFiles) = scapsWorkingSet.getElements.toList.map {
       case javaProject: IJavaProject =>
         val classPath = extractClassPath(javaProject)
         val projectSourceFragmentRoots = javaProject.getAllPackageFragmentRoots.filter(_.getKind == IPackageFragmentRoot.K_SOURCE).toList
@@ -70,9 +71,9 @@ class ScapsIndexService(private val scapsAdapter: ScapsAdapter) extends StrictLo
       case unknownTyp =>
         logger.info("type not supported: " + unknownTyp.getClass)
         returnElements(None, None, None)
-    }).unzip3
+    }.unzip3
 
-    (collectedElements._1.flatten.distinct, collectedElements._2.flatten.distinct, collectedElements._3.flatten.distinct)
+    (classPaths.flatten.distinct, projectSourceFragmentRoots.flatten.distinct, librarySourceRootFiles.flatten.distinct)
   }
 
   private def indexProjectTask(monitor: IProgressMonitor, classPath: List[String], projectSourceFragmentRoots: List[IPackageFragmentRoot]): Unit = {

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/core/services/ScapsService.scala

@@ -24,15 +24,17 @@ import org.eclipse.core.resources.ResourcesPlugin
 
 object ScapsService {
 
-  val pluginPreferences = InstanceScope.INSTANCE.getNode(ScapsPlugin.PLUGIN_ID)
-  val indexRootDir = ResourcesPlugin.getWorkspace.getRoot.getLocation.toString + ScapsPlugin.INDEX_RELATIVE_ROOT_DIR
+  private val pluginPreferences = InstanceScope.INSTANCE.getNode(ScapsPlugin.PLUGIN_ID)
+  private val indexRootDir = ResourcesPlugin.getWorkspace.getRoot.getLocation.toString + ScapsPlugin.INDEX_RELATIVE_ROOT_DIR
 
   val PROPERTY_INDEXER_RUNNING = "indexRunning"
 
   val PROPERTY_SEARCH_ON_FIRST_INDEX = "searchOnFirstIndex"
   val FIRST_INDEX_DIR = "first"
   val SECOND_INDEX_DIR = "second"
 
+  // Könnte es hier zu Threading-Problemen kommen? Race condition, etc.?
+
   private def isSearchOnFirstIndex = pluginPreferences.getBoolean(PROPERTY_SEARCH_ON_FIRST_INDEX, true)
   private def searchIndexDir = if (isSearchOnFirstIndex) FIRST_INDEX_DIR else SECOND_INDEX_DIR
   private def indexingIndexDir = if (isSearchOnFirstIndex) SECOND_INDEX_DIR else FIRST_INDEX_DIR
@@ -52,6 +54,7 @@ object ScapsService {
     pluginPreferences.getBoolean(PROPERTY_INDEXER_RUNNING, false)
   }
 
+  // Entspricht nicht den Scala-Namenskonventionen. Wie wärs mit createSearchService und createIndexService?
   def SEARCH = {
     val indexDir = indexRootDir + searchIndexDir
     val scapsAdapter = new ScapsAdapter(indexDir)

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/handlers/IndexUCHandler.scala

@@ -13,6 +13,7 @@ import scaps.eclipse.core.services.ScapsIndexService
 import scaps.eclipse.core.services.ScapsService
 
 object IndexUCHandler {
+  // Bringt diese Methode etwas?
   private def INSTANCE = new IndexUCHandler(ScapsService.INDEXING)
   def apply(): IndexUCHandler = INSTANCE
 }

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/handlers/SearchUCHandler.scala

@@ -9,11 +9,12 @@ import scaps.api.ValueDef
 import scaps.eclipse.core.services.ScapsSearchService
 
 object SearchUCHandler {
+  // Wozu?
   private def INSTANCE = new SearchUCHandler(ScapsService.SEARCH)
   def apply(): SearchUCHandler = INSTANCE
 }
 
-class SearchUCHandler(private val scapsSearchService: ScapsSearchService) {
+class SearchUCHandler(scapsSearchService: ScapsSearchService) {
 
   def apply(searchQuery: String): Seq[Result[ValueDef]] = scapsSearchService(searchQuery)
 

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/actions/SampleAction.scala

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+// Lizenz?
+
 package scaps.eclipse.ui.view.actions
 
 import java.io.File
@@ -34,15 +36,9 @@ import scaps.scala.featureExtraction.ScalaSourceExtractor
 import scaps.searchEngine.SearchEngine
 import scaps.settings.Settings
 
-/**
- * Our sample action implements workbench action delegate.
- * The action proxy will be created by the workbench and
- * shown in the UI. When the user tries to use the action,
- * this delegate will be created and execution will be
- * delegated to it.
- * @see IWorkbenchWindowActionDelegate
- */
+// Rename?
 class SampleAction extends IWorkbenchWindowActionDelegate with StrictLogging {
+  // Needed?
   private var window: IWorkbenchWindow = _
 
   /**
@@ -78,7 +74,7 @@ class SampleAction extends IWorkbenchWindowActionDelegate with StrictLogging {
     val scalaSrcFiles = srcFiles.filter(!_.getName.endsWith(".scala"))
     printEachFile(scalaSrcFiles)
 
-    var conf = Settings.fromApplicationConf.modIndex { index => index.copy(indexDir = workspacePath + "/.metadata/scaps") }
+    val conf = Settings.fromApplicationConf.modIndex { index => index.copy(indexDir = workspacePath + "/.metadata/scaps") }
     val engine = SearchEngine(conf).get
     engine.resetIndexes
 
@@ -182,4 +178,4 @@ class SampleAction extends IWorkbenchWindowActionDelegate with StrictLogging {
   def init(window: IWorkbenchWindow) {
     this.window = window;
   }
-}
+}

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/actions/ScapsSearchAction.scala

@@ -12,30 +12,24 @@ class ScapsSearchAction extends IWorkbenchWindowActionDelegate {
 
   private var window: IWorkbenchWindow = _
 
-  def init(window: IWorkbenchWindow) {
+  def init(window: IWorkbenchWindow): Unit = {
     this.window = window
   }
 
-  def run(action: IAction) {
-    getWorkbenchWindow
+  def run(action: IAction): Unit = {
     if (window.getActivePage == null) {
+      // Error handling
       print("Run: Something is not good!")
       return
     }
     NewSearchUI.openSearchDialog(window, ScapsPlugin.SEARCH_PAGE)
   }
 
-  def selectionChanged(action: IAction, selection: ISelection) {
+  def selectionChanged(action: IAction, selection: ISelection): Unit = {
   }
 
-  def dispose {
+  // Bei Methoden mit Seiteneffekten ist die Konvention () zu schreiben
+  def dispose(): Unit = {
     window = null
   }
-
-  def getWorkbenchWindow {
-    if (window == null) {
-      window = SearchPlugin.getActiveWorkbenchWindow
-    }
-  }
-
 }

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/dialogs/ProjectSelectionDialog.scala

@@ -27,6 +27,7 @@ import org.eclipse.core.resources.ResourcesPlugin
 import org.eclipse.jface.viewers.CheckStateChangedEvent
 import org.eclipse.swt.events.SelectionEvent
 
+// Format on multiple lines?
 class ProjectSelectionDialog(var window: IWorkbenchWindow, var selected: Array[IProject]) extends MessageDialog(window.getShell, "Project Indexer", null, "The project indexer allows you to use the Scaps Search", MessageDialog.NONE, Array("Index now", "Cancel"), 0) {
 
   private var projectNames: CheckboxTableViewer = _
@@ -41,11 +42,13 @@ class ProjectSelectionDialog(var window: IWorkbenchWindow, var selected: Array[I
     if (buttonId != IDialogConstants.OK_ID) {
       return
     }
+    // Wozu?
     //save all dirty editors
     BuildUtilities.saveEditors(null);
     //batching changes ensures that autobuild runs after cleaning
   }
 
+  // Cleanup needed
   override def createCustomArea(parent: Composite): Control = {
     val area = new Composite(parent, SWT.NONE)
     area.setFont(parent.getFont)
@@ -166,10 +169,6 @@ class ProjectSelectionDialog(var window: IWorkbenchWindow, var selected: Array[I
     content
   }
 
-  override def close(): Boolean = {
-    super.close()
-  }
-
   override def getInitialLocation(initialSize: Point): Point = {
     new Point(50, 50)
   }

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/handlers/ScapsAssistantHandler.scala

@@ -13,6 +13,6 @@ class ScapsAssistantHandler extends AbstractHandler {
       window.getShell,
       "ScalaSearchIDE",
       "Scaps Assistant")
-    return null;
+    null
   }
-}
+}

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/search/ScapsSearchPage.scala

@@ -44,6 +44,7 @@ class ScapsSearchPage extends DialogPage with ISearchPage with StrictLogging {
     exampleQueriesLabel.setText("Example Queries")
 
     val exampleQueriesTextLabel = new Label(result, SWT.NONE)
+    // Hier könnte man einen Multi-Line String mit """xx""" nehmen, der kann Zeilenumbrüche enthalten.
     exampleQueriesTextLabel.setText("max: Int - An integer value with `max` in it's name or doc comment \n max: (Int, Int) => Int - A function taking two ints and returning Int. \n max: Int => Int => Int - Same query as above but in curried form. \n Ordering[String] - Implementations of the `Ordering` typeclass for strings. \n List[A] => Int => Option[A] - A generic query which uses a type parameter `A`. All type identifiers consisting of a single character are treated as type parameters. \n List => Int => Option - The identical query as above but with omitted type parameters. \n &~ - Searches for symbolic operators are also possible.")
 
     setControl(result)

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/search/ScapsSearchResult.scala

@@ -16,6 +16,7 @@ import scaps.api.ValueDef
  */
 class ScapsSearchResult(query: ScapsSearchQuery) extends ISearchResult {
 
+  // Sehr unschön, eventuell in Getter/Setter kapseln? Nicht viel besser, doofes API.
   var data: Seq[Result[ValueDef]] = _
 
   /**

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/search/ScapsSearchResultLabelProvider.scala

@@ -78,6 +78,7 @@ class ScapsSearchResultLabelProvider extends StyledCellLabelProvider with Strict
     val text = new StyledString
 
     cell.getElement match {
+      // TODO fix Result(entity: ValueDef)
       case result: Result[ValueDef @unchecked] =>
         cell.setImage(ScalaImages.SCALA_FILE.createImage)
         text.append(result.entity.name)

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/search/ScapsSearchResultPage.scala

@@ -49,6 +49,7 @@ class ScapsSearchResultPage extends AbstractTextSearchViewPage with StrictLoggin
     labelProvider.getScapsDocHTML(backgroundColor, foregroundColor)(_)
   }
 
+  // Könnte man in eigene Klasse extrahieren, sieht ein bisschen hässlich aus hier:
   private val treeContentProvider = new ITreeContentProvider() {
     def dispose(): Unit = {}
     def inputChanged(viewer: Viewer, x: Any, y: Any): Unit = {}
@@ -107,11 +108,20 @@ class ScapsSearchResultPage extends AbstractTextSearchViewPage with StrictLoggin
     tableViewer.setContentProvider(contentProvider)
     tableViewer.setLabelProvider(labelProvider)
 
-    tableViewer.addSelectionChangedListener(new ISelectionChangedListener() {
+    tableViewer.addSelectionChangedListener(new ISelectionChangedListener {
       def selectionChanged(event: SelectionChangedEvent): Unit = {
         for {
           selection <- Option(event.getSelection).collect { case s: StructuredSelection => s }
-          result <- Option(selection.getFirstElement).collect { case r: Result[ValueDef @unchecked] => r }
+          /* Ich habe das mal refactored um das @unchecked weg zu kriegen,
+           * alelrdings ist der Code sehr unschön. Allerdings wird später im
+           * scapsDocProvider soweit ich sehe nur die entity gebraucht, man
+           * könnte also stattdessen nur das übergeben, das ginge dann so:
+           *
+           *  collect { case Result(v: ValueDef, _, _) => v }
+           *
+           * und sieht wieder hübscher aus.
+           *  */
+          result <- Option(selection.getFirstElement).collect { case r @ Result(_: ValueDef, _, _) => r.asInstanceOf[Result[ValueDef]] }
         } {
           val shell = tableViewer.getControl.getShell
           val scapsDocHTML = scapsDocProvider(result)
@@ -136,7 +146,7 @@ class ScapsSearchResultPage extends AbstractTextSearchViewPage with StrictLoggin
             result.entity.source match {
               case fileSource: FileSource =>
                 val artifactPath = fileSource.artifactPath
-                val relativePath = artifactPath.substring(ResourcesPlugin.getWorkspace.getRoot.getLocation.toString().length())
+                val relativePath = artifactPath.substring(ResourcesPlugin.getWorkspace.getRoot.getLocation.toString.length)
                 val path = new Path(relativePath)
                 val file = ResourcesPlugin.getWorkspace.getRoot.getFile(path)
 

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/workingset/ScapsWorkingSetPage.scala

@@ -19,15 +19,15 @@ class ScapsWorkingSetPage extends JavaWorkingSetPage {
 
     // get the working set name text control and disable it
     for {
-      subComposite <- composite.getChildren.collect { case c: Composite => c }.headOption
-      workingSetNameText <- subComposite.getChildren.collect { case t: Text => t }.headOption
+      subComposite <- composite.getChildren.collectFirst { case c: Composite => c }
+      workingSetNameText <- subComposite.getChildren.collectFirst { case t: Text => t }
     } {
       workingSetNameText.setEditable(false)
       workingSetNameText.setEnabled(false)
     }
 
     for {
-      subComposite <- composite.getChildren.collect { case c: Composite => c }.headOption
+      subComposite <- composite.getChildren.collectFirst { case c: Composite => c }
     } {
       performBuildCheckbox = createPerformBuildCheckbox(subComposite)
     }

scala-ide-scaps-plugin/src/main/scala/scaps/eclipse/ui/view/workingset/ScapsWorkingSetPageContentProvider.scala

@@ -12,17 +12,19 @@ class ScapsWorkingSetPageContentProvider extends StandardJavaElementContentProvi
   import StandardJavaElementContentProvider.NO_CHILDREN
 
   override def hasChildren(element: Any): Boolean = element match {
-    case javaProject: IJavaProject => true
-    case _                         => false
+    case _: IJavaProject => true
+    case _               => false
   }
 
   override def getChildren(parentElement: Any): Array[Object] = parentElement match {
-    case javaModel: IJavaModel => super.getChildren(parentElement)
-    case javaProject: IJavaProject => super.getChildren(parentElement).filter(_ match {
-      case library: JarPackageFragmentRoot => library.getSourceAttachmentPath != null
-      case p: PackageFragmentRoot          => true
-      case _                               => false
-    })
+    case _: IJavaModel =>
+      super.getChildren(parentElement)
+    case _: IJavaProject =>
+      super.getChildren(parentElement).filter {
+        case library: JarPackageFragmentRoot => library.getSourceAttachmentPath != null
+        case _: PackageFragmentRoot          => true
+        case _                               => false
+      }
     case _ => NO_CHILDREN
   }