Create a new scope in `collect` body

[Yardanico]

Fixes a part of #19287, previously collect didn't create a new scope so all variables defined inside of collect call were available outside.

Also changed newTree(nnkStmtList) to newStmtList for shorter code

[ghost]

I'm confused about newBlockStmt - since in this case, we need the block to return a value should we maybe use nnkBlockExpr instead? But I don't see any info about it.

[metagn]

I'm confused about newBlockStmt - since in this case, we need the block to return a value should we maybe use nnkBlockExpr instead? But I don't see any info about it.

I don't think there's a semantic difference between them (this should be documented if it hasn't already), so you can't use them to error if it doesn't return an expression. They are just reflective of the syntax the parser encounters. For example this compiles and runs fine:

(if true:
  echo "a"
else:
  echo "b")

But is parsed as:

StmtListExpr
  IfStmt
    ElifExpr
      Ident "true"
      StmtList
        Command
          Ident "echo"
          StrLit "a"
    ElseExpr
      StmtList
        Command
          Ident "echo"
          StrLit "b"

In this case using Expr helps describe what the macro is outputting, but in general I wouldn't use the Expr variants for macro outputs as they do not seem to be the "default" versions. But it doesn't matter.

[Araq]

The reason it was done this way is that you can easily introduce a new scope via block on your own, but you cannot easily remove it when the macro does it for you. It's not a bug.

[SolitudeSF]

it looks like a bug, and i bet no one would expect it not to open new scope.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

[konsumlamm]

The reason it was done this way is that you can easily introduce a new scope via block on your own, but you cannot easily remove it when the macro does it for you.

If you want to use variables from inside the collect: block, you can just declare them outside of it, so I don't really buy this argument.

[github-actions[bot]]

This pull request is stale because it has been open for 1 year with no activity. Contribute more commits on the pull request and rebase it on the latest devel, or it will be closed in 30 days. Thank you for your contributions.

[github-actions[bot]]

This pull request has been marked as stale and closed due to inactivity after 395 days.