arnetheduck / nph

An opinionated code formatter for Nim
Other
77 stars 12 forks source link

respect the '.' in chained methods. #28

Closed FullValueRider closed 5 months ago

FullValueRider commented 5 months ago

I had the following code

for myStr in s.Data:
    var myNums: string = myStr
    myNums = 
      myNums
        .replace("one", "o1ne")
        .replace("two", "t2wo")
        .replace("three", "t3hree")
        .replace("four", "f4our")
        .replace("five", "f5ive")
        .replace("six", "s6ix")
        .replace("seven", "s7even")
        .replace("eight", "e8ight")
        .replace("nine", "n9ine")
        .replace("zero", "z0ero")
        .strip(chars = myLower)

after applying nph it became the mess below.

for myStr in s.Data:
    var myNums: string = myStr
    mynums =
      myNums.replace("one", "o1ne").replace("two", "t2wo").replace("three", "t3hree").replace(
        "four", "f4our"
      ).replace("five", "f5ive").replace("six", "s6ix").replace("seven", "s7even").replace(
        "eight", "e8ight"
      ).replace("nine", "n9ine").replace("zero", "z0ero").strip(chars = myLower)

To my mind this raises a number of issues

  1. Line lengths should be adjusted to the nearest . prior to the end of the line where chained methods are distinct (below 'replacen' is used to emulate distinct methods)
    mynums =
      myNums.replace1("one", "o1ne").replace2("two", "t2wo").replace3("three", "t3hree")
         .replace5("four", "f4our").replace5("five", "f5ive").replace6("six", "s6ix").replace7("seven", "s7even")

    and not

    mynums =
      myNums.replace("one", "o1ne").replace("two", "t2wo").replace("three", "t3hree").replace(
        "four", "f4our"
      ).replace("five", "f5ive").replace("six", "s6ix").replace("seven", "s7even").replace(
  2. chained methods should vertically align at the '.' when the current method is the same as the previous method.
    .replace("nine", "n9ine").replace("zero", "z0ero")

    should become

    .replace("nine", "n9ine")
    .replace("zero", "z0ero")
  3. if the previous two chained methods are the same then the current chained method should be on its own line if a different method.
    .replace("nine", "n9ine")
    .replace("zero", "z0ero")
    .strip(chars = myLower)

    not

    .replace("nine", "n9ine")
    .replace("zero", "z0ero").strip(chars = myLower)

Please note that the sequence of replaces was due to the code being a first translation effort from VBA. Please also note the multireplace was not used because the targets were overlapping and multireplace in nim cannot cope with overlapping targets. (e.g. nineight). This code has subsequently refactored such that the replaces were encapsulates ina seq and iterated over. The seq statement was

let mySwaps = @[ ("one", "o1ne"), ("two", "t2wo"),("three", "t3hree"),,("four", "f4our"),("five", "f5ive"),("six", "s6ix"),("seven", "s7even"),(eight", "e8ight"),,("nine", "n9ine"),("zero", "z0ero")]

which was nicely formatted to

let
    mySwaps =
      @[
        ("one", "o1ne"),
        ("two", "t2wo"),
        ("three", "t3hree"),
        ("four", "f4our"),
        ("five", "f5ive"),
        ("six", "s6ix"),
        ("seven", "s7even"),
        ("eight", "e8ight"),
        ("nine", "n9ine"),
        ("zero", "z0ero")
      ]