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

[WONTFIX] nimpretty infers wrong indentation #9512

Closed
timotheecour opened this issue Oct 25, 2018 · 7 comments
Closed

[WONTFIX] nimpretty infers wrong indentation #9512

timotheecour opened this issue Oct 25, 2018 · 7 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Oct 25, 2018

proc foo_verylong_verylong_verylong_verylong_verylong_verylong_verylong(a1 = 1,
    a2 = "asdfasdf", a3 = "asdfasdf"): auto =
  discard

nimpretty indents as:

proc foo_verylong_verylong_verylong_verylong_verylong_verylong_verylong(
        a1 = 1,
    a2 = "asdfasdf", a3 = "asdfasdf"): auto =
    discard

(ie, it wrongly inferred indentation of 4, whereas it was 2)

EDIT: s/margin/indentation/ (was a typo)

@Araq
Copy link
Member

Araq commented Oct 25, 2018

It doesn't infer it at all, that's why. ;-)

@timotheecour
Copy link
Member Author

timotheecour commented Oct 25, 2018

it does:
this diff:

diff --git a/compiler/layouter.nim b/compiler/layouter.nim
index 9f9b6fc8e..fe6125d7e 100644
--- a/compiler/layouter.nim
+++ b/compiler/layouter.nim
@@ -43,6 +43,7 @@ proc openEmitter*(em: var Emitter, cache: IdentCache;
   let fullPath = Absolutefile config.toFullPath(fileIdx)
   em.indWidth = getIndentWidth(fileIdx, llStreamOpen(fullPath, fmRead),
                                cache, config)
+  echo ("em.indWidth:", em.indWidth)
   if em.indWidth == 0: em.indWidth = 2
   em.config = config
   em.fid = fileIdx

produces this output:

(Field0: "em.indWidth:", Field1: 4)
so getIndentWidth infers 4, when it should've inferred 2 in this example; somehow it got "tricked" by the argument reflow

a not-very-robust way to infer in this example is to take the 1st argument reflow (line 2: 4 spaces) and divide by 2; however that's only ok if it was obtained by a previous invocation of nimpretty;
a more robust way is to infer by ignoring argument reflow and infer it from 1st indentation it finds after that (line 3=>2 spaces)

@Araq
Copy link
Member

Araq commented Oct 25, 2018

It infers the indentation, not the "margin".

@timotheecour timotheecour changed the title nimpretty infers wrong margin nimpretty infers wrong indentation Oct 25, 2018
@timotheecour
Copy link
Member Author

timotheecour commented Oct 25, 2018

duh... was a typo; i meant indentation; the bug still holds despite my typo :)

@timotheecour timotheecour changed the title nimpretty infers wrong indentation [WONTFIX] nimpretty infers wrong indentation Feb 13, 2019
@dom96
Copy link
Contributor

dom96 commented Feb 13, 2019

Why close this? It seems like a genuine bug

@timotheecour
Copy link
Member Author

timotheecour commented Feb 13, 2019

@dom96 because of this comment from @narimiran #10629 (comment)

Btw, sorry to be blunt, but I will not consider/review your PRs until the count of your open issues is above 150. Close some least important ones (which affect the least amount of Nim users), this will show some good will from your side, and then I'll change my mind.

I went through all my open issues and they're all valid (IMO), but I had to reduce to < 150 to avoid this stalemate.

@krux02
Copy link
Contributor

krux02 commented Feb 14, 2019

@timotheecour Please show some respect. You are supposed to reduce your issues to set priorities on them. They are not all valid. I am sure I can find some that are invalid or written in a very obscure way that I have no idea what the real problem is. But the point is, you should do it and don't continue to waste our time on it. Only keep the important and issues open and close the issues that are not important. Then it would be really nice of you, when from the other 144 issues if you would check if they conform the issues template and rework them to make them easier processable. You still have 144 issues from 1256 issues in total. That is more that 10% from you alone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants