Cleanup code for review

Author: wenli-cai

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/App.kt

@@ -167,16 +167,14 @@ internal fun App(
       )
     }
 
-    if (traceMode is TraceMode.Live) {
-      if ((traceMode as TraceMode.Live).device == null) {
-        DisplayDevices(
-          onDeviceSelect = { selectedDevice ->
-            traceMode = TraceMode.Live(selectedDevice)
-          },
-          devices = listDevices(),
-          modifier = Modifier.align(Alignment.Center)
-        )
-      }
+    if (traceMode is TraceMode.Live && (traceMode as TraceMode.Live).device == null) {
+      DisplayDevices(
+        onDeviceSelect = { selectedDevice ->
+          traceMode = TraceMode.Live(selectedDevice)
+        },
+        devices = listDevices(),
+        modifier = Modifier.align(Alignment.Center)
+      )
 
       FileDump(
         trace = rawRenderPass,
@@ -186,8 +184,7 @@ internal fun App(
 
     RightInfoPanel(
       selectedNode = selectedNode,
-      modifier = Modifier
-        .align(Alignment.TopEnd)
+      modifier = Modifier.align(Alignment.TopEnd)
     )
   }
 }

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/SocketClient.kt

@@ -11,6 +11,10 @@ import kotlinx.coroutines.withContext
 import okio.IOException
 import java.net.Socket
 
+/**
+ * Maintains the channel, which acts as buffer for information read from the socket, as the data are
+ * being processed.
+ */
 internal suspend fun streamRenderPassesFromDevice(device: String, parseOnNewRenderPass: (String) -> Unit) {
   val renderPassChannel: Channel<String> = Channel(Channel.BUFFERED)
   coroutineScope {

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/parser/DiffUtils.kt

@@ -20,8 +20,8 @@ fun computeAnnotatedDiff(
     .showInlineDiffs(true)
     .inlineDiffByWord(true)
     .mergeOriginalRevised(true)
-    .oldTag { f -> "--" }
-    .newTag { f -> "++" }
+    .oldTag { _ -> "--" }
+    .newTag { _ -> "++" }
     .build()
 
   val pastName = extractTypeName(past)
@@ -40,9 +40,6 @@ fun computeAnnotatedDiff(
         text = "$pastName(...)",
         builder = this
       )
-      // pushStyle(DiffStyles.DELETE)
-      // append("$pastName(...)")
-      // pop()
       append(" → ")
       buildStringWithStyle(
         style = DiffStyles.INSERT,
@@ -202,8 +199,8 @@ private fun getFieldsAsList(field: String): List<String> {
         if (depth == 0) { // end of key=value pair
           fields += currentField.toString().trim()
           currentField.clear()
-          i++ // skip space, e.g. "key=value, key2=value2, etc..."
-        } else { // nested list
+          i++ // skip space in between key value pairs, i.e. "key1=value1, key2=value2"
+        } else { // nested list, so we ignore
           currentField.append(char)
         }
       }

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/parser/JsonParser.kt

@@ -7,16 +7,14 @@ import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
 import com.squareup.workflow1.traceviewer.model.Node
 import com.squareup.workflow1.traceviewer.model.addChild
 import com.squareup.workflow1.traceviewer.model.replaceChild
-import com.squareup.workflow1.traceviewer.util.parser.ParseResult.Failure
-import com.squareup.workflow1.traceviewer.util.parser.ParseResult.Success
 import io.github.vinceglb.filekit.PlatformFile
 import io.github.vinceglb.filekit.readString
 import kotlin.reflect.jvm.javaType
 import kotlin.reflect.typeOf
 
 /*
  The root workflow Node uses an ID of 0, and since we are filtering childrenByParent by the
- parentId, the root node has a parent of -1 ID. This is reflected seen inside android-register
+ parentId, the root node will have -1 as a fake parent ID. This is reflected inside android-register.
  */
 const val ROOT_ID: String = "-1"
 
@@ -33,11 +31,11 @@ internal suspend fun parseFileTrace(
   val jsonString = file.readString()
   val workflowAdapter = createMoshiAdapter<List<Node>>()
   val parsedRenderPasses = try {
-    workflowAdapter.fromJson(jsonString) ?: return Failure(
+    workflowAdapter.fromJson(jsonString) ?: return ParseResult.Failure(
       IllegalArgumentException("Provided trace file is empty or malformed.")
     )
   } catch (e: Exception) {
-    return Failure(e)
+    return ParseResult.Failure(e)
   }
 
   val parsedFrames = parsedRenderPasses.map { renderPass -> getFrameFromRenderPass(renderPass) }
@@ -47,7 +45,7 @@ internal suspend fun parseFileTrace(
     frameTrees.add(mergedTree)
     mergedTree
   }
-  return Success(
+  return ParseResult.Success(
     trace = parsedFrames,
     trees = frameTrees,
     affectedNodes = parsedRenderPasses
@@ -66,11 +64,11 @@ internal fun parseLiveTrace(
   currentTree: Node? = null
 ): ParseResult {
   val parsedRenderPass = try {
-    adapter.fromJson(renderPass) ?: return Failure(
+    adapter.fromJson(renderPass) ?: return ParseResult.Failure(
       IllegalArgumentException("Provided trace data is empty or malformed.")
     )
   } catch (e: Exception) {
-    return Failure(e)
+    return ParseResult.Failure(e)
   }
 
   val parsedFrame = getFrameFromRenderPass(parsedRenderPass)
@@ -83,7 +81,7 @@ internal fun parseLiveTrace(
   }
 
   // Since live tracing handles one frame at a time, we generalize and return listOf for each.
-  return Success(
+  return ParseResult.Success(
     trace = listOf(parsedFrame),
     trees = listOf(mergedTree),
     affectedNodes = listOf(parsedRenderPass)
@@ -155,9 +153,13 @@ internal fun mergeFrameIntoMainTree(
 }
 
 internal sealed interface ParseResult {
-  class Success(val trace: List<Node>, val trees: List<Node>, affectedNodes: List<List<Node>>) :
-    ParseResult {
+  class Success(
+    val trace: List<Node>,
+    val trees: List<Node>,
+    affectedNodes: List<List<Node>>
+  ) : ParseResult {
     val affectedNodes = affectedNodes.map { it.toSet() }
   }
+
   class Failure(val error: Throwable) : ParseResult
 }

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/parser/TraceParser.kt

@@ -72,7 +72,7 @@ internal fun RenderTrace(
           tree = parseResult.trees,
           affected = parseResult.affectedNodes
         )
-        // Only increment the frame index and add the raw data during Live tracing mode
+        // Only increment the frame index and add the raw data during Live tracing mode.
         onNewFrame?.invoke()
         rawRenderPass?.let { onNewData(it) }
       }
@@ -107,14 +107,15 @@ internal fun RenderTrace(
 
   // This will only happen in the initial switch to Live Mode, where a socket error bubbled up and
   // the lambda call to parse the data was immediately cancelled, meaning handleParseResult was never
-  // called to set isLoading to false
+  // called to set isLoading to false.
   if (isLoading && error != null) {
     Text("Socket Error: $error")
     return
   }
 
   // This meant that there was an exception, but it was stored in ParseResult and read in
-  // handleParseResult. Since there is no parsed data, this likely means there was a moshi parsing error
+  // handleParseResult method. Since there is no parsed data, this likely means there was a moshi
+  // parsing error.
   if (error != null && frames.isEmpty()) {
     Text("Malformed File: $error")
     return