search

nim-lang / bigints

BigInts for Nim
MIT License
123 stars 32 forks source link

Split bigints.nim in multiple files #114

Closed dlesnoff closed 1 year ago

dlesnoff commented 2 years ago

I have splitted bigints.nim into multiple logical files and reordered some functions, those operating on limbs went into initBigInt.nim file with the type definition and all initBigInt procedures.

There's one slight problem with this refactoring. Since all files are included in about the same order in bigints.nim, the nim compiler understands only bigints.nim file. It tricks the LSP that believe there is multiple errors on all files succeding initBigInt.nim.

One work around would be to include only one last file in initBigInt.nim, and all files include the previous. I don't like this solution because we can not see the list of files really included at the end in initBigInt file.

Right now, debugging will be significantly harder (LSP helps me to detect errors before compiling!). Tell me if the split is ok for you, i.e. if the name files and the splitting seems logical.

The diff is hard to read, but there is nothing we can do about it.

juancarlospaco commented 1 year ago

It should use import instead of include IMHO.

dlesnoff commented 1 year ago

Thank you @juancarlospaco for your comment. I fear that import will be hard to handle with all the mutual dependencies of the program. I will need to begin from scratch to do so, and add a lot of forward declarations, but I agree that might be better in the end.

juancarlospaco commented 1 year ago

But then is not much of an improvement as-is...

konsumlamm commented 1 year ago

Hmm, I don't really like this organization.

I agree that it would be nice to split off some parts into their own files. I think the first step would be to create something like bigints/core.nim, which only contains the BigInt definition. However, that's already a problem, since then the other modules couldn't access the private fields...

Including the files is a bad idea if it breaks IDE tooling.

dlesnoff commented 1 year ago

However, that's already a problem, since then the other modules couldn't access the private fields... That is the reason why I have gone for includes rather than import then. I see no possible fix so I close this issue.