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

Fix various warnings seen when running nimble test on 1.6.10 #157

Merged
merged 1 commit into from
May 11, 2023

Conversation

marcusramberg
Copy link
Contributor

Fixes about a million warnings like this by running nimpretty:

/Users/marcus/Source/nimlsp/src/nimlsppkg/messages.nim(592, 25) Warning: Number of spaces around '?:' is not consistent [Spacing]

as well as:

/Users/marcus/Source/nimlsp/src/nimlsppkg/messages.nim(2, 8) Warning: imported and not used: 'messageenums' [UnusedImport]
/Users/marcus/Source/nimlsp/tests/tnimlsp.nim(1, 11) Warning: imported and not used: 'streams' [UnusedImport]
/Users/marcus/Source/nimlsp/tests/tnimlsp.nim(1, 11) Warning: imported and not used: 'asyncfile' [UnusedImport]

@PMunch
Copy link
Owner

PMunch commented Apr 10, 2023

Looks good. The spacing issue was done intentionally though, to make it look similar to TypeScript. Annoying that Nim complains about it though, so I guess I can accept this. Or we could throw ignore spacing issues pragmas around that part.

@marcusramberg
Copy link
Contributor Author

Looks good. The spacing issue was done intentionally though, to make it look similar to TypeScript. Annoying that Nim complains about it though, so I guess I can accept this. Or we could throw ignore spacing issues pragmas around that part.

I had a suspicion it might be intentional. I can update my PR if you prefer an ignore pragma instead.

@marcusramberg
Copy link
Contributor Author

Looks good. The spacing issue was done intentionally though, to make it look similar to TypeScript. Annoying that Nim complains about it though, so I guess I can accept this. Or we could throw ignore spacing issues pragmas around that part.

I tested

{.warning[Spacing]:off.}

around the offending code in messages.nim - but it's still throwing warnings, so not sure how to disable it. 😞

@PMunch
Copy link
Owner

PMunch commented May 2, 2023

I think you need to use {.push.} and {.pop.}? But I might be wrong, I don't usually disable warnings

@marcusramberg
Copy link
Contributor Author

I think you need to use {.push.} and {.pop.}? But I might be wrong, I don't usually disable warnings

I tried that variant as well without luck. Maybe it's better to just let it use the recommended formatting though?

@PMunch PMunch merged commit c235b25 into PMunch:master May 11, 2023
11 of 12 checks passed
@marcusramberg marcusramberg deleted the marcus/fix_warnings branch May 11, 2023 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants