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

styleCheck: Fix error for sugar and std/with #16176

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

ee7
Copy link
Contributor

@ee7 ee7 commented Nov 28, 2020

With this commit, we no longer see an error if we pass
--styleCheck:error when compiling a file that contains import sugar
or import std/with.

The problem was that those modules (and only those modules) import
std/private/underscored_calls, which contained a styleCheck issue:
its spelling of nnkArgList didn't match the nnkArglist spelling in
macros.nim.

This commit fixes the issue by renaming nnkArglist to nnkArgList
repo-wide.

Fixes: #16174

CI failure is unrelated (see #16169).

@ringabout ringabout closed this Nov 30, 2020
@ringabout ringabout reopened this Nov 30, 2020
@ee7 ee7 force-pushed the styleCheck-fix-error-sugar-std-with branch from 9dc3db1 to 5fdd98f Compare November 30, 2020 12:58
@ee7
Copy link
Contributor Author

ee7 commented Nov 30, 2020

@xflywind Closing and reopening this PR doesn't help here to make CI green :)

This PR needed to be rebased on devel so that it includes the commit dd5405c.

@Araq
Copy link
Member

Araq commented Dec 1, 2020

This is a breaking change though for --styleCheck:error based code. Instead rename nkArgList to nkArglist.

With this commit, we no longer see an error if we pass
`--styleCheck:error` when compiling a file that contains `import sugar`
or `import std/with`.

The problem was that those modules (and only those modules) import
`std/private/underscored_calls`, which contained a styleCheck issue:
its spelling of `nnkArgList` didn't match the `nnkArglist` spelling in
`macros.nim`.

This commit fixes the issue by renaming `nnkArgList` to `nnkArglist`
repo-wide. The other way around would be a breaking change for code that
used `nnkArglist` and `--styleCheck:error`.

Fixes: nim-lang#16174
@ee7 ee7 force-pushed the styleCheck-fix-error-sugar-std-with branch from 5fdd98f to 188e360 Compare December 2, 2020 19:18
@ee7
Copy link
Contributor Author

ee7 commented Dec 2, 2020

This is a breaking change though for --styleCheck:error based code.

Ok - I've updated this PR so that it does nnkArgList -> nnkArglist repo-wide, including in

of nnkTypeDef(_, _, nnkTypeClassTy(nnkArgList, _, _, nnkStmtList)):

but maybe you'd prefer to leave that one as-is.

Instead rename nkArgList to nkArglist

Did you mean to write nnk, or do you also want nkArgList -> nkArglist?

nnkArgList -> nnkArglist does seem a bit unfortunate, given that we have:

  • nnkStmtList
  • nnkStmtListExpr
  • nnkStmtListType
  • nnkRecList
  • nkArgList (one n)

I'm okay with the current state of this PR, given that it fixes import sugar with --styleCheck:error.

But is it not possible to switch to nnkArgList in macros.nim, adding a special case to styleCheck so that compiling a file containing nnkArglist with --styleCheck:error produces a deprecation warning, rather than an error?

@Araq
Copy link
Member

Araq commented Dec 2, 2020

I meant it in the way you did it.

@Araq Araq merged commit 629b22e into nim-lang:devel Dec 2, 2020
@ee7 ee7 deleted the styleCheck-fix-error-sugar-std-with branch December 2, 2020 20:39
mildred pushed a commit to mildred/Nim that referenced this pull request Jan 11, 2021
With this commit, we no longer see an error if we pass
`--styleCheck:error` when compiling a file that contains `import sugar`
or `import std/with`.

The problem was that those modules (and only those modules) import
`std/private/underscored_calls`, which contained a styleCheck issue:
its spelling of `nnkArgList` didn't match the `nnkArglist` spelling in
`macros.nim`.

This commit fixes the issue by renaming `nnkArgList` to `nnkArglist`
repo-wide. The other way around would be a breaking change for code that
used `nnkArglist` and `--styleCheck:error`.

Fixes: nim-lang#16174
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
With this commit, we no longer see an error if we pass
`--styleCheck:error` when compiling a file that contains `import sugar`
or `import std/with`.

The problem was that those modules (and only those modules) import
`std/private/underscored_calls`, which contained a styleCheck issue:
its spelling of `nnkArgList` didn't match the `nnkArglist` spelling in
`macros.nim`.

This commit fixes the issue by renaming `nnkArgList` to `nnkArglist`
repo-wide. The other way around would be a breaking change for code that
used `nnkArglist` and `--styleCheck:error`.

Fixes: nim-lang#16174
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.

styleCheck: Errors for import sugar and import std/with
3 participants