Remove re-definition of SomeSet
#34
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit ebee42e ("A little cleanup around set types.", 2021-10-01) added an exported definition of `SomeSet` that included the built-in `set` type. However, a typical Nim user is familiar with the definition of `SomeSet` in the standard library of SomeSet*[A] = HashSet[A] | OrderedSet[A] and was thus likely to read the lines proc parseHook*[T](s: string, i: var int, v: var SomeSet[T]) = and proc dumpHook*[T](s: var string, v: SomeSet[T]) = and believe that jsony does not support the built-in `set` type. This commit therefore removes the `SomeSet` re-definition, which also means that jsony once again works with code like import std/sets import pkg/jsony proc foo[T](s: SomeSet[T]) = discard which successfully compiled with jsony 1.0.5 and earlier, but produced an error with jsony 1.1.0 (2021-11-04): /tmp/foo.nim(4, 16) Error: ambiguous identifier: 'SomeSet' -- use one of the following: sets.SomeSet: SomeSet jsony.SomeSet: SomeSet Support for the built-in `set` type was originally added by commit 9a4f118 (2021-10-01). Fixes: treeform#33
|
Thank you for the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit ebee42e ("A little cleanup around set types.", 2021-10-01)
added an exported definition of
SomeSetthat included thebuilt-in
settype.However, a typical Nim user is familiar with the definition of
SomeSetin the standard library of
and was thus likely to read the lines
and
and believe that jsony does not support the built-in
settype.This commit therefore removes the
SomeSetre-definition, which alsomeans that jsony once again works with code like
which successfully compiled with jsony 1.0.5 and earlier, but produced
an error with jsony 1.1.0 (2021-11-04):
Support for the built-in
settype was originally added bycommit 9a4f118 (2021-10-01).
Fixes: #33