nim-lang / RFCs

A repository for your Nim proposals.
137 stars 23 forks source link

Allow to indent `of/else/elif/except/finally` for block commands #420

Closed haxscramper closed 3 years ago

haxscramper commented 3 years ago

Proposal is pretty simple - allow additional level of indentation for command calls:

  match arg:
    of test1: discard
    of test2: discard
    elif 12: discard
    except: discard
    else: discard

This topic came up several times in discussion of caseStmtMacros. Main reason for this change - there should be only one case in the language, and implicitly overloading it with multiple different semantics is not the best idea

Instead, being able to write a custom macro that looks like case is much easier, but right now case allows for indented of, but command block doesn't. This is probably the only limitation, it is pretty minor, but still annoying because of random inconsistencies.

Related

Araq commented 3 years ago

The way to write the case statement is:

case variable
of value1, value2: ...

No additional indentation and no colon after variable. The alternative was allowed because some users demanded it but it never made sense as no "code block" follows after the (optional) colon.

Likewise we only need to support

  match arg
  of test1: discard
  of test2: discard
  elif 12: discard
  except: discard
  else: discard

IMO.

But I still like the overloaded case better.

haxscramper commented 3 years ago
match arg:
  myof ...

match arg:
of ...

case arg:
of ..

case arg:
  of ...

Are allowed, but

match arg:
  of ...

Is not allowed. If we disregard stylistic preferences about number of spaces before 'of', optionally of the colon, we still have this inconsistency where parsing of the 'of' branches differs between case statement and user commands. Both allow to use branches, but one supports indent, and other does not.

Also,

match arg:
  myof ...

Is the only way to write it, because

match arg:
myof ...

Is not valid.

haxscramper commented 3 years ago

But I still like the overloaded case better.

What about the reasoning for the customMacro that I provided? This RFC is directly tied to it, so I think it can be addressed as well.

disruptek commented 3 years ago

How about, instead, deprecating case expr: syntax; it promotes a mental model that doesn't match reality and I cannot see how this is advantageous to someone learning the language.

haxscramper commented 3 years ago

Okay, got it, stylistic preferences etc. Don't think there is any reason to keep this RFC open then, probably won't be accepted anyway.

haxscramper commented 3 years ago

Why reopen? It seems like most responses would be "no, because I don't like to indent 'of'" (i.e. this will turn into stylistic holywar without addressing any other points).

Araq commented 3 years ago

Well it's not a bad idea in itself. I merely happen to dislike it but not strongly enough. Plus my own proposal (match x\nof...) does not work with today's parser either.

haxscramper commented 3 years ago

Got it. I reopened PR then, its CI is green and fix itself is ~7 lines, so it is mostly a yes/no decision in the RFC about whether we want this in the language.

juancarlospaco commented 3 years ago

I am neutral, but I want Pattern Matching more close to StdLib, even if it has to be a more simplified version of it.

haxscramper commented 3 years ago

Pattern matching in stdlib also should wait for (1) let expressions, (2) stable view types, and some reimplementation effort for fusion/matching. This issue is not directly blocking std/matching ... it really just an absolutely tiny inconsistency that I think could be fixed, that's all. Most languages with pattern matching allow to indent arms, and case can be indented, so IMO it would be weird to explicitly disallow this, or make it work somehow differently.

Varriount commented 3 years ago

I'd prefer pattern matching to be available as a non-standard library module first, and then later incorporated into the standard library. That way we have some definite evidence regarding which design or approach works best.

haxscramper commented 3 years ago

@Varriount and that's exactly why I decided to add pattern matching to fusion first, so everyone could test it out and see what is working and what is not. Over the past 11 months since I made my PR https://github.com/nim-lang/fusion/pull/33 I didn't have any major complaints with design, things that people did complain about were mostly about missing functionality, or lack of docs for certain parts, which I also tried to fix in timely manner. Overall, I think my specification from #245 worked well enough, with the only noticeable inconvenience being that there can be only one implementation of the patter matching in a given scope, and it would implicitly change the semantics of the built-in language construct based on the switch expression type. Which is, IMO, quite questionable.

Also, if you have any specific complaints about pattern matching implementation you can open the issue in fusion, or comment on #245, or tag me on any of the real-time chats (that's where a noticeable portion of fusion improvements were designed).

haxscramper commented 3 years ago

I also did make a PR https://github.com/nim-lang/Nim/pull/17047 into stdlib when fusion was unbundled from nim, but was told (in discord discussion that followed) that "well if you want it in the standard, it should look exactly as we want it", (in context of let expressions https://irclogs.nim-lang.org/15-02-2021.html#18:49:45). I agree that we need to wait for let expressions, and some other things, and after all I decided to close the PR because "current implementation can be improved in certain areas, and regardless whether we would get it into stdlib at some point, it would differ from this PR" https://github.com/nim-lang/Nim/pull/17047#issuecomment-884336453

And while view types and let expressions are relatively huge, 'match' syntax requires 7 line fix in the compiler, and would make it possible to write macros that provide completely seamless syntactic interoperability with 'case'. Without any new features, implicit type-based change-semantics-of-basic-constructs, must-use-experimental-feature solutions.

Araq commented 3 years ago

What would a parser fix that allows for match x\nof take?

haxscramper commented 3 years ago

Just to make sure, you mean this, right?

match arg
of test

Well in my PR I just expand postExprBlocks with trivial check for indentation. In case of match arg ... it should be able to handle all the edge cases, so command statement parsing should look ahead, check if there is an of, and parse it, otherwise finish parsing. At least that's what I would probably try first, maybe it would require more look ahead, or checking around to support all the syntax variations.

  match arg1, arg2
  of 1: discard

  match arg1 = test
  of 1: discard

This can probably be done, though I don't understand why we need to get rid of colon delimiter for this, since every single block statement uses it - block/try/catch/except/finally/if/else/elif/when/of, {.pragma.} section. And macro/template/proc block calls also used this syntax.

Also, how it should treat

myMacro 123
myOf 12: discard

I imagine it should be the same, or somehow considered as well?