nushell / nufmt

MIT License
64 stars 8 forks source link

Cover flatshapes #35

Closed AucaCoyan closed 8 months ago

AucaCoyan commented 1 year ago

All right! That took some time 🕙. Sorry for the long wait, I had life coming in the middle. Anywho, I finished a bit more of FlatShapes

There are a bunch more to solve, but most importantly, the code is screaming for a refactor. The issue is that I used more and more booleans to do quick checks "is this coming from a def keyword?" "are we in a string interpolation line?" and so on, that it gets very spaguetti if we dont use smarter solutions like peek for example

As a conclusion, I am ready for this PR, but I know now that the direction is not to include all the remaining FlatShapes, but reorganize the code a little better 😇

AucaCoyan commented 1 year ago

Currently, the nufmt has to strip all the whitespace and breaklines as possible, so then we can work into indentations and proper line length. I started with good code so I can see what it nufmt does with it. The most important thing in all of the examples is that nufmt does not break code! 🪂

I'm advancing, I can give a few examples: 1️⃣ from this file

export def-env goto [
    query?: string  # a search query to narrow down the list of choices
] {
    let choice = (
        pick repo 
        $"Please (
        ansi yellow_italic)choose a repo(ansi reset) to (ansi green_underline)jump to:(ansi reset)"
        $query
    )
    if ($choice | is-empty) { return }
    cd (get root dir | path join $choice)
}

nufmt outputs

export def-env goto [
    query?: string  # a search query to narrow down the list of choices
] { let choice = ( pick repo $"Please (ansi yellow_italic )choose a repo(ansi reset ) to (ansi green_underline )jump to:(ansi reset )"$queryy) 
if ( $choice | is-empty ) { return } 
cd ( get root dir | path join $choice ) } 

sample 2️⃣ :

# fuzzy-delete any repository managed by `gm`
export def remove [
    query?: string      # a search query to narrow down the list of choices
    --force (-f): bool  # do not ask for comfirmation when deleting a repository
] {
    let choice = (pick repo
        $"Please (ansi yellow_italic)choose a repo(ansi reset) to (ansi red_underline)completely remove:(ansi reset)"
        $query
    )
    if ($choice | is-empty) {
        return
    }

    let repo = (get root dir | path join $choice)
    if $force {
        rm --trash --verbose --recursive $repo
    } else {
        rm --trash --verbose --recursive $repo --interactive
    }
}

to this

# fuzzy-delete any repository managed by `gm`
export def remove [
    query?: string      # a search query to narrow down the list of choices
    --force (-f): bool  # do not ask for comfirmation when deleting a repository
] { 
let choice = ( pick repo $"Please (ansi yellow_italic )choose a repo(ansi reset ) to (ansi red_underline )completely remove:(ansi reset )"$query ) 
if ( $choice | is-empty ) { return } 
let repo = ( get root dir | path join $choice ) 
if $force { rm --trash --verbose --recursive $repo } else { rm --trash --verbose --recursive $repo --interactive } } 

and the final 3️⃣ :

# get windows service status
def get-win-svc [] {
  sc queryex type=service state=all | 
    collect {|x| $x | 
    parse -r '(?m)SERVICE_NAME:\s*(?<svc>\w*)\s*DISPLAY_NAME:\s*(?<dsp>.*)\s*TYPE\s*:\s*(?<type>[\da-f]*)\s*(?<typename>\w*)?\s*\s*STATE\s*:\s*(?<state>\d)\s*(?<status>\w*)\s*(?<state_opts>\(.*\))?\s*?WIN32_EXIT_CODE\s*:\s*(?<exit>\d*).*\s*SERVICE_EXIT_CODE\s*:\s*(?<svc_exit>\d)\s*.*\s*CHECKPOINT\s*:\s*(?<chkpt>.*)\s*WAIT_HINT\s*:\s(?<wait>.*)\s*PID\s*:\s*(?<pid>\d*)\s*FLAGS\s*:\s(?<flags>.*)?' | 
    upsert status {|s| 
      if $s.status == RUNNING { 
        $"(ansi green)●(ansi reset)" 
      } else { 
        $"(ansi red)●(ansi reset)" 
      }
    } | into int state exit svc_exit chkpt wait pid
    }
}

and the result

# get windows service status
def get-win-svc [] { sc queryex type=service state=all | collect {|x | $x | parse -r '(?m)SERVICE_NAME:\s*(?<svc>\w*)\s*DISPLAY_NAME:\s*(?<dsp>.*)\s*TYPE\s*:\s*(?<type>[\da-f]*)\s*(?<typename>\w*)?\s*\s*STATE\s*:\s*(?<state>\d)\s*(?<status>\w*)\s*(?<state_opts>\(.*\))?\s*?WIN32_EXIT_CODE\s*:\s*(?<exit>\d*).*\s*SERVICE_EXIT_CODE\s*:\s*(?<svc_exit>\d)\s*.*\s*CHECKPOINT\s*:\s*(?<chkpt>.*)\s*WAIT_HINT\s*:\s(?<wait>.*)\s*PID\s*:\s*(?<pid>\d*)\s*FLAGS\s*:\s(?<flags>.*)?' | upsert status {|s| 
      if $s.status == RUNNING { 
        $"(ansi green)●(ansi reset)" 
      } else { 
        $"(ansi red)●(ansi reset)" 
      }
    } | into int state exit svc_exit chkpt wait pid } } 

with the final example I realized some problems, the FlatShape::Closure is not working as intended. The nufmt inputs and outputs almost the same, which not the expected result. Also, the software still needs some tuning to add spaces in the correct order, but those are details in comparison to other breaking issues. Step by step we are moving on!

amtoine commented 1 year ago

@AucaCoyan

let choice = ( pick repo $"Please (ansi yellow_italic )choose a repo(ansi reset ) to (ansi red_underline )completely remove:(ansi reset )"$query )

this does not look right to me :thinking: the $query being right next to the end of the previous string :confused:

also in the first example, i assume the double y in $queryy is just a bad copy-paste?

Step by step we are moving on!

good to hear that :relieved:

AucaCoyan commented 1 year ago

the $query being right next to the end of the previous string 😕 also in the first example, i assume the double y in $queryy is just a bad copy-paste?

Yes, that doesn't look great. We need to dial in some details. queryy was a typo 😋

amtoine commented 1 year ago

Yes, that doesn't look great. We need to dial in some details.

at least if it's still correct :crossed_fingers:

queryy was a typo yum

glad to hear that :wink:

amtoine commented 1 year ago

@AucaCoyan looks like you've pushed commits from the main branch here, haven't you? :open_mouth:

AucaCoyan commented 8 months ago

if new tests reflecting these new flatshapes [or new tests] should be added?

Yes! the existing tests reflect every of these combinations. Before we didn't "looked into" closures and definitions, but now that we do, a lot of asserts went bad on the first run. A couple of lines after, I made the changes to see CI in green 😄

sholderbach commented 8 months ago

I think we shouldn't stall experimental progress here so the DeclId should not strictly block this but we need to make a change on the nu side soon so this is not breaking on changes in nushell.

AucaCoyan commented 8 months ago

Hello! Hope you are doing well 😄 This PR got a little stalled imo. What do you think about merging this and start with the new design of the upcoming parser? I don't want to hurry at all! but I think we have arrived to a stalemate among DeclId, FlatShape::KeywordCall and the new parser, what do you think?

amtoine commented 8 months ago

agree @AucaCoyan, let's move forward with this :relieved:

AucaCoyan commented 8 months ago

Thank you so much! This was an enormous rock for me in this project. This PR waited months. I finally did it 😭😀

fdncred commented 8 months ago

wow, a 6 month long pr finally landed. good job!