Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nimpretty wrong indentation for doc comments #10156

Closed
GULPF opened this issue Jan 1, 2019 · 4 comments

Comments

@GULPF
Copy link
Member

commented Jan 1, 2019

Nimpretty doesn't fix indentation for doc comments except for the first line.

Before

proc foo =
    ## Comment
    ## Comment
    discard

proc bar =
  ## Comment
  ## Comment
  discard

After

proc foo =
    ## Comment
    ## Comment
    discard

proc bar =
    ## Comment
  ## Comment
    discard

Expected after

proc foo =
    ## Comment
    ## Comment
    discard

proc bar =
    ## Comment
    ## Comment
    discard

@GULPF GULPF added the nimpretty label Jan 1, 2019

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Jan 1, 2019

@GULPF the code you posted is invalid:

Error: implementation of 't0094.bar()[declared in t0094.nim(9, 6)]' expected

not clear to me nimpretty should handle correctly invalid code (or at least, it's not as urgent as fixing all the other issues with nimpretty on valid code)

## proposal:
precede nimpretty by nim check foo, and fail if nim check fails

@GULPF

This comment has been minimized.

Copy link
Member Author

commented Jan 1, 2019

@timotheecour It's not invalid, they are forward declarations. This compiles:

proc foo
    ## Comment
    ## Comment

proc bar
  ## Comment
  ## Comment

proc foo = discard
proc bar = discard

This bug happens for non-forward declarations as well.

precede nimpretty by nim check foo, and fail if nim check fails

Not sure that's a good idea. There's no reason why code that doesn't survive semcheck can't be formatted.

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Jan 1, 2019

still, the code you posted in top-post is invalid; can you update it with the version that's valid from #10156 (comment) ?
(and yes, your bug is indeed a nimpretty bug)

@timotheecour

This comment has been minimized.

Copy link
Contributor

commented Jan 1, 2019

There's no reason why code that doesn't survive semcheck can't be formatted

semcheck, maybe, but it should parse at least; is there a way to check for that?
rationale: the code will reformat in un-intended way otherwise, it's much safer to abort

EDIT: looks like it's already the case:

proc main()=
  let a = 1
  when defined(foo):
    a.inc
   echo a

main()

nimpretty correctly bails with: Error: invalid indentation

@Araq Araq closed this in 9f82e07 Jun 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.