tighten rhs expression whitespace and indent

When rendering the right-hand expression of a keyword like `return` or an assignment, sometimes we would add a newline and indent. This PR avoids such extra indent/newlines by allowing partial expressions such as function calls to continue on the same line like so: ```nim let myvariable = functioncall( arg0, arg1, ... ) ``` This saves significant amounts of space as we avoid the extra indent that previously would apply to the arguments in this case.
arnetheduck · Feb 4, 2024 · 5b9ee02 · 5b9ee02
1 parent dc5e13c
commit 5b9ee02
Show file tree

Hide file tree

Showing 14 changed files with 447 additions and 335 deletions.
diff --git a/docs/src/style.md b/docs/src/style.md
@@ -192,3 +192,42 @@ spaces around these infixes and sometimes not, leading to irregularity.
 Since there's no consensus in existing code at the time of writing, the rule is
 irregular and causes implementation complexity, `nph` formats `..` and `..<`
 with spaces.
+
+## Expressions
+
+Expressions appear in many places, such as after certain keywords (`return`,
+`yield`), as part of control flows (`if`, `while`), in assignments etc.
+
+Whenever possible, `nph` will try to keep the full expression on a single line:
+
+```nim
+let myvariable = somelongexpression(abc)
+```
+
+If this is not possible, the second preference is to move the whole expression
+to a new line, assuming it fits:
+
+```nim
+let myvariable =
+   someevenlongerexpression(abc, def)
+```
+
+If the expression still doesn't fit, we'll split it up on multiple lines:
+
+```nim
+let myvariable = someevenlongerexpression(
+  aaa, bbb, ccc, ddd
+)
+```
+
+Certain expressions linked by related keywords that don't fit on a single line
+will also be moved to a new line - for example, a multi-line `if`/`else` nested
+in a `return` will be lined up like so:
+
+```nim
+return
+  if condition:
+    complex(call)
+  else:
+    alsocomplex(call)
+```
diff --git a/src/phast.nim b/src/phast.nim
@@ -1407,22 +1407,20 @@ const PackageModuleId* = -3'i32
 proc idGeneratorFromModule*(m: PSym): IdGenerator =
   assert m.kind == skModule
 
-  result =
-    IdGenerator(
-      module: m.itemId.module,
-      symId: m.itemId.item,
-      typeId: 0,
-      disambTable: initCountTable[PIdent](),
-    )
+  result = IdGenerator(
+    module: m.itemId.module,
+    symId: m.itemId.item,
+    typeId: 0,
+    disambTable: initCountTable[PIdent](),
+  )
 
 proc idGeneratorForPackage*(nextIdWillBe: int32): IdGenerator =
-  result =
-    IdGenerator(
-      module: PackageModuleId,
-      symId: nextIdWillBe - 1'i32,
-      typeId: 0,
-      disambTable: initCountTable[PIdent](),
-    )
+  result = IdGenerator(
+    module: PackageModuleId,
+    symId: nextIdWillBe - 1'i32,
+    typeId: 0,
+    disambTable: initCountTable[PIdent](),
+  )
 
 proc nextSymId*(x: IdGenerator): ItemId {.inline.} =
   assert(not x.sealed)
@@ -1655,18 +1653,17 @@ proc newSym*(
 
   let id = nextSymId idgen
 
-  result =
-    PSym(
-      name: name,
-      kind: symKind,
-      flags: {},
-      info: info,
-      itemId: id,
-      options: options,
-      owner: owner,
-      offset: defaultOffset,
-      disamb: getOrDefault(idgen.disambTable, name).int32,
-    )
+  result = PSym(
+    name: name,
+    kind: symKind,
+    flags: {},
+    info: info,
+    itemId: id,
+    options: options,
+    owner: owner,
+    offset: defaultOffset,
+    disamb: getOrDefault(idgen.disambTable, name).int32,
+  )
 
   idgen.disambTable.inc name
   when false:
@@ -1685,8 +1682,8 @@ proc astdef*(s: PSym): PNode =
     s.ast
 
 proc isMetaType*(t: PType): bool =
-  return t.kind in tyMetaTypes or (t.kind == tyStatic and t.n == nil) or
-      tfHasMeta in t.flags
+  return
+    t.kind in tyMetaTypes or (t.kind == tyStatic and t.n == nil) or tfHasMeta in t.flags
 
 proc isUnresolvedStatic*(t: PType): bool =
   return t.kind == tyStatic and t.n == nil
@@ -1863,15 +1860,14 @@ proc `$`*(s: PSym): string =
     result = "<nil>"
 
 proc newType*(kind: TTypeKind, id: ItemId, owner: PSym): PType =
-  result =
-    PType(
-      kind: kind,
-      owner: owner,
-      size: defaultSize,
-      align: defaultAlignment,
-      itemId: id,
-      uniqueId: id,
-    )
+  result = PType(
+    kind: kind,
+    owner: owner,
+    size: defaultSize,
+    align: defaultAlignment,
+    itemId: id,
+    uniqueId: id,
+  )
 
   when false:
     if result.itemId.module == 55 and result.itemId.item == 2:
@@ -2065,16 +2061,15 @@ proc delSon*(father: PNode, idx: int) =
 template transitionNodeKindCommon(k: TNodeKind) =
   let obj {.inject.} = n[]
 
-  n[] =
-    TNode(
-      kind: k,
-      typ: obj.typ,
-      info: obj.info,
-      flags: obj.flags,
-      prefix: obj.prefix,
-      mid: obj.mid,
-      postfix: obj.postfix,
-    )
+  n[] = TNode(
+    kind: k,
+    typ: obj.typ,
+    info: obj.info,
+    flags: obj.flags,
+    prefix: obj.prefix,
+    mid: obj.mid,
+    postfix: obj.postfix,
+  )
   # n.comment = obj.comment # shouldn't be needed, the address doesnt' change
   when defined(useNodeIds):
     n.id = obj.id
@@ -2100,24 +2095,23 @@ proc transitionNoneToSym*(n: PNode) =
 template transitionSymKindCommon*(k: TSymKind) =
   let obj {.inject.} = s[]
 
-  s[] =
-    TSym(
-      kind: k,
-      itemId: obj.itemId,
-      magic: obj.magic,
-      typ: obj.typ,
-      name: obj.name,
-      info: obj.info,
-      owner: obj.owner,
-      flags: obj.flags,
-      ast: obj.ast,
-      options: obj.options,
-      position: obj.position,
-      offset: obj.offset,
-      loc: obj.loc,
-      annex: obj.annex,
-      constraint: obj.constraint,
-    )
+  s[] = TSym(
+    kind: k,
+    itemId: obj.itemId,
+    magic: obj.magic,
+    typ: obj.typ,
+    name: obj.name,
+    info: obj.info,
+    owner: obj.owner,
+    flags: obj.flags,
+    ast: obj.ast,
+    options: obj.options,
+    position: obj.position,
+    offset: obj.offset,
+    loc: obj.loc,
+    annex: obj.annex,
+    constraint: obj.constraint,
+  )
 
   when hasFFI:
     s.cname = obj.cname

diff --git a/src/phoptions.nim b/src/phoptions.nim
@@ -475,10 +475,9 @@ type
     suggestMaxResults*: int
     lastLineInfo*: TLineInfo
     writelnHook*: proc(output: string) {.closure, gcsafe.}
-    structuredErrorHook*:
-      proc(config: ConfigRef, info: TLineInfo, msg: string, severity: Severity) {.
-        closure, gcsafe
-      .}
+    structuredErrorHook*: proc(
+      config: ConfigRef, info: TLineInfo, msg: string, severity: Severity
+    ) {.closure, gcsafe.}
     cppCustomNamespace*: string
     nimMainPrefix*: string
     vmProfileData*: ProfileData
@@ -628,58 +627,57 @@ proc initConfigRefCommon(conf: ConfigRef) =
   conf.mainPackageNotes = NotesVerbosity[1]
 
 proc newConfigRef*(): ConfigRef =
-  result =
-    ConfigRef(
-      cCompiler: ccGcc,
-      macrosToExpand: newStringTable(modeStyleInsensitive),
-      arcToExpand: newStringTable(modeStyleInsensitive),
-      m: initMsgConfig(),
-      cppDefines: initHashSet[string](),
-      headerFile: "",
-      features: {},
-      legacyFeatures: {},
-      configVars: newStringTable(modeStyleInsensitive),
-      symbols: newStringTable(modeStyleInsensitive),
-      packageCache: newPackageCache(),
-      searchPaths: @[],
-      lazyPaths: @[],
-      outFile: RelativeFile"",
-      outDir: AbsoluteDir"",
-      prefixDir: AbsoluteDir"",
-      libpath: AbsoluteDir"",
-      nimcacheDir: AbsoluteDir"",
-      dllOverrides: newStringTable(modeCaseInsensitive),
-      moduleOverrides: newStringTable(modeStyleInsensitive),
-      cfileSpecificOptions: newStringTable(modeCaseSensitive),
-      projectName: "", # holds a name like 'nim'
-      projectPath: AbsoluteDir"", # holds a path like /home/alice/projects/nim/compiler/
-      projectFull: AbsoluteFile"", # projectPath/projectName
-      projectIsStdin: false, # whether we're compiling from stdin
-      projectMainIdx: FileIndex(0'i32), # the canonical path id of the main module
-      command: "", # the main command (e.g. cc, check, scan, etc)
-      commandArgs: @[], # any arguments after the main command
-      commandLine: "",
-      keepComments: true, # whether the parser needs to keep comments
-      implicitImports: @[], # modules that are to be implicitly imported
-      implicitIncludes: @[], # modules that are to be implicitly included
-      docSeeSrcUrl: "",
-      cIncludes: @[], # directories to search for included files
-      cLibs: @[], # directories to search for lib files
-      cLinkedLibs: @[], # libraries to link
-      backend: backendInvalid,
-      externalToLink: @[],
-      linkOptionsCmd: "",
-      compileOptionsCmd: @[],
-      linkOptions: "",
-      compileOptions: "",
-      ccompilerpath: "",
-      toCompile: @[],
-      arguments: "",
-      suggestMaxResults: 10_000,
-      maxLoopIterationsVM: 10_000_000,
-      vmProfileData: newProfileData(),
-      spellSuggestMax: spellSuggestSecretSauce,
-    )
+  result = ConfigRef(
+    cCompiler: ccGcc,
+    macrosToExpand: newStringTable(modeStyleInsensitive),
+    arcToExpand: newStringTable(modeStyleInsensitive),
+    m: initMsgConfig(),
+    cppDefines: initHashSet[string](),
+    headerFile: "",
+    features: {},
+    legacyFeatures: {},
+    configVars: newStringTable(modeStyleInsensitive),
+    symbols: newStringTable(modeStyleInsensitive),
+    packageCache: newPackageCache(),
+    searchPaths: @[],
+    lazyPaths: @[],
+    outFile: RelativeFile"",
+    outDir: AbsoluteDir"",
+    prefixDir: AbsoluteDir"",
+    libpath: AbsoluteDir"",
+    nimcacheDir: AbsoluteDir"",
+    dllOverrides: newStringTable(modeCaseInsensitive),
+    moduleOverrides: newStringTable(modeStyleInsensitive),
+    cfileSpecificOptions: newStringTable(modeCaseSensitive),
+    projectName: "", # holds a name like 'nim'
+    projectPath: AbsoluteDir"", # holds a path like /home/alice/projects/nim/compiler/
+    projectFull: AbsoluteFile"", # projectPath/projectName
+    projectIsStdin: false, # whether we're compiling from stdin
+    projectMainIdx: FileIndex(0'i32), # the canonical path id of the main module
+    command: "", # the main command (e.g. cc, check, scan, etc)
+    commandArgs: @[], # any arguments after the main command
+    commandLine: "",
+    keepComments: true, # whether the parser needs to keep comments
+    implicitImports: @[], # modules that are to be implicitly imported
+    implicitIncludes: @[], # modules that are to be implicitly included
+    docSeeSrcUrl: "",
+    cIncludes: @[], # directories to search for included files
+    cLibs: @[], # directories to search for lib files
+    cLinkedLibs: @[], # libraries to link
+    backend: backendInvalid,
+    externalToLink: @[],
+    linkOptionsCmd: "",
+    compileOptionsCmd: @[],
+    linkOptions: "",
+    compileOptions: "",
+    ccompilerpath: "",
+    toCompile: @[],
+    arguments: "",
+    suggestMaxResults: 10_000,
+    maxLoopIterationsVM: 10_000_000,
+    vmProfileData: newProfileData(),
+    spellSuggestMax: spellSuggestSecretSauce,
+  )
 
   initConfigRefCommon(result)
   setTargetFromSystem(result.target)
@@ -947,27 +945,26 @@ proc getNimcacheDir*(conf: ConfigRef): AbsoluteDir =
 proc pathSubs*(conf: ConfigRef, p, config: string): string =
   let home = removeTrailingDirSep(os.getHomeDir())
 
-  result =
-    unixToNativePath(
-      p % [
-        "nim",
-        getPrefixDir(conf).string,
-        "lib",
-        conf.libpath.string,
-        "home",
-        home,
-        "config",
-        config,
-        "projectname",
-        conf.projectName,
-        "projectpath",
-        conf.projectPath.string,
-        "projectdir",
-        conf.projectPath.string,
-        "nimcache",
-        getNimcacheDir(conf).string
-      ]
-    ).expandTilde
+  result = unixToNativePath(
+    p % [
+      "nim",
+      getPrefixDir(conf).string,
+      "lib",
+      conf.libpath.string,
+      "home",
+      home,
+      "config",
+      config,
+      "projectname",
+      conf.projectName,
+      "projectpath",
+      conf.projectPath.string,
+      "projectdir",
+      conf.projectPath.string,
+      "nimcache",
+      getNimcacheDir(conf).string
+    ]
+  ).expandTilde
 
 iterator nimbleSubs*(conf: ConfigRef, p: string): string =
   let pl = p.toLowerAscii