[Nim] Bfbs Nim Generator by danlapid · Pull Request #7534 · google/flatbuffers

danlapid · 2022-09-15T21:11:28Z

Based on the existing languages (mostly - swift, python, lua) and on https://github.com/RecruitMain707/NimFlatbuffers
Relevant issue: #5855
Replaces: #7362

So I had more time to work on it and seeing as how @dbaileychess requested I use the bfbs format instead I though it'd be wiser to just start with a clean branch and add only what's relevant.
Also, in accordance with @CasperN 's suggestions I submit this PR without OneFile nor Object Api which can be added in a different PR at a later date.

I hope we can start iterating comments over this PR and add this great language in!

…to nim_bfbs

danlapid · 2022-09-23T15:07:32Z

indeed, anything that can be done about it?

Pinging would work, but also submitting a PR also works - the restriction is only for first time contributors. The fix to Type::Serialize seems important to think about and a good candidate to split out into a small PR.

I doubt I will need many more rounds of ci before this works (hopefully next run will work, the general tasks are passing, only my new test nim task is giving me issues that I hope are solved).
Considering that I'd rather not indulge in another PR unless you deem it a necessary split.

danlapid · 2022-09-23T20:20:36Z

@CasperN @dbaileychess I’d love a ci run whenever you’re available, Thanks

…to nim_bfbs

danlapid · 2022-09-27T10:24:08Z

@dbaileychess @CasperN
Seems like the CI is finally green (currently yellow just because I merged with master to keep the branch up to date).
Any other comments regarding the implementation?

danlapid · 2022-10-19T13:37:15Z

@CasperN Pulled and merged with master, no conflicts and tests are passing.
Anything else we want to change regarding this PR?

dbaileychess · 2022-10-21T01:16:08Z

@danlapid I would like to include this in the next version release. Looks like @CasperN had one open converstation to resolve.

danlapid · 2022-10-21T07:03:16Z

@danlapid I would like to include this in the next version release. Looks like @CasperN had one open converstation to resolve.

I’ll take care of it today and ping you guys

danlapid · 2022-10-21T08:34:08Z

@CasperN @dbaileychess Added the requested code.

CasperN

Hi, I still have a minor comment and question about where certain code should live

danlapid · 2022-10-21T14:00:43Z

@CasperN Fixed the issues you noted in your last 2 comments.

CasperN · 2022-10-21T18:29:53Z

LGTM!

* Bfbs Nim Generator * Remove commented out tests * add missing line to idl.h * Commit python reflection changes * Commit python reflection changes and move tests * Remove default string addition * Move tests to python file * Fix element size check when element is table * remove whitespace changes * add element_type docs and commit further to namer and remove kkeep * Bfbs Nim Generator * Remove commented out tests * add missing line to idl.h * Commit python reflection changes * Commit python reflection changes and move tests * Remove default string addition * Move tests to python file * Fix element size check when element is table * remove whitespace changes * add element_type docs and commit further to namer and remove kkeep * remove unused variables * added tests to ci * added tests to ci * fixes * Added reflection type Field, Variable to namer * Moved reflection namer impl to bfbsnamer * Remove whitespace at end of line * Added nim to generated code * Revert whitespace removal Co-authored-by: Derek Bailey <derekbailey@google.com>

Bfbs Nim Generator

9cfefce

github-actions bot added c++ CI Continuous Integration codegen Involving generating code from schema labels Sep 15, 2022

danlapid added 3 commits September 16, 2022 00:13

Remove commented out tests

a739546

add missing line to idl.h

8e0b4c1

Commit python reflection changes

7b09a80

github-actions bot added the python label Sep 15, 2022

dbaileychess reviewed Sep 16, 2022

View reviewed changes

Comment thread CMakeLists.txt Outdated

Comment thread reflection/reflection.fbs Outdated

Comment thread tests/FlatBuffers.Test.Nim/tests/moredefaults/test.nim

Comment thread tests/FlatBuffers.Test.Nim/NimTest.sh Outdated

Comment thread nim/flatbuffers.nimble Outdated

danlapid added 5 commits September 16, 2022 11:22

Commit python reflection changes and move tests

92ac716

Remove default string addition

61e2ae3

Merge branch 'nim_bfbs' of https://github.com/danlapid/flatbuffers in…

37e4f4c

…to nim_bfbs

Move tests to python file

bf1dcfc

Fix element size check when element is table

14c639a

github-actions bot added the lua label Sep 16, 2022

danlapid commented Sep 16, 2022

View reviewed changes

Comment thread tests/MyGame/Example/Monster.lua

remove whitespace changes

581ffe1

CasperN reviewed Sep 19, 2022

View reviewed changes

Comment thread src/bfbs_gen.h

Comment thread src/bfbs_gen_nim.cpp Outdated

danlapid added 11 commits September 20, 2022 13:44

add element_type docs and commit further to namer and remove kkeep

a5278bd

Bfbs Nim Generator

cfba8fd

Remove commented out tests

c7e8bbf

add missing line to idl.h

c432b95

Commit python reflection changes

b6279dd

Commit python reflection changes and move tests

fcb84c0

Remove default string addition

6fdf6ae

Move tests to python file

92c736e

Fix element size check when element is table

6e6969b

remove whitespace changes

8ea3fc8

add element_type docs and commit further to namer and remove kkeep

1e16326

danlapid force-pushed the nim_bfbs branch from a5278bd to 1e16326 Compare September 22, 2022 19:24

danlapid added 3 commits September 24, 2022 14:06

Merge branch 'nim_bfbs' of https://github.com/danlapid/flatbuffers in…

dd88e3e

…to nim_bfbs

Merge branch 'nim_bfbs' of https://github.com/danlapid/flatbuffers in…

e27f0c4

…to nim_bfbs

Merge branch 'master' into nim_bfbs

011c237

CasperN reviewed Sep 27, 2022

View reviewed changes

Comment thread src/bfbs_gen_nim.cpp Outdated

Comment thread src/bfbs_gen_nim.cpp

Comment thread src/bfbs_gen_nim.cpp Outdated

Comment thread src/bfbs_gen_nim.cpp Outdated

Comment thread src/bfbs_gen_nim.cpp Outdated

Comment thread src/bfbs_gen_nim.cpp

danlapid added 3 commits September 27, 2022 18:59

fixes

a8c97e1

Merge branch 'master' into nim_bfbs

dbf21aa

merge

67a5556

dbaileychess added the release-notes The PR should be highlighted in the release notes of the next release it is in. label Oct 19, 2022

danlapid added 2 commits October 21, 2022 11:10

Merge branch 'master' into nim_bfbs

bdf5e59

Added reflection type Field, Variable to namer

38eef94

CasperN reviewed Oct 21, 2022

View reviewed changes

Comment thread src/namer.h Outdated

Comment thread tests/nim/testnim.py Outdated

danlapid added 4 commits October 21, 2022 16:30

Moved reflection namer impl to bfbsnamer

aa3fd9e

Remove whitespace at end of line

368fd21

Added nim to generated code

39aeae1

Revert whitespace removal

e6dacdf

Merge branch 'master' into nim_bfbs

c01cd09

dbaileychess approved these changes Oct 21, 2022

View reviewed changes

CasperN merged commit 872a497 into google:master Oct 21, 2022

Conversation

danlapid commented Sep 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danlapid commented Sep 23, 2022

Uh oh!

danlapid commented Sep 23, 2022

Uh oh!

danlapid commented Sep 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danlapid commented Oct 19, 2022

Uh oh!

dbaileychess commented Oct 21, 2022

Uh oh!

danlapid commented Oct 21, 2022

Uh oh!

danlapid commented Oct 21, 2022

Uh oh!

CasperN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danlapid commented Oct 21, 2022

Uh oh!

CasperN commented Oct 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants