SciNim / nim-plotly

plotly wrapper for nim-lang
https://scinim.github.io/nim-plotly/
MIT License
181 stars 15 forks source link

make code more Nim idiomatic #6

Closed Vindaar closed 6 years ago

Vindaar commented 6 years ago

Since I had some free time and you mentioned that you're still relatively new to Nim, I thought I'd change the code somewhat to make it more idiomatic Nim style, i.e. apply the style guide https://nim-lang.org/docs/nep1.html and the style of the standard library. I may be doing some non idiomatic things myself, so feel free to criticize (or just ignore it if you don't simply don't like it :) ).

What I changed:

Note: this PR is based on PR #4, so that should be merged before hand (if you decide to merge this anyways ;) ). I hope you don't mind me doing this. :)

ghost commented 6 years ago

About "introduced blocks of let, var, const where applicable" - it's not really specified in the NEP-1, I also talked with dom96 about this - it's just a personal preference whether you should use blocks of var/let statements or not. But it's fine :)

ghost commented 6 years ago

Why did you change some procs to func though?

Vindaar commented 6 years ago

I simply started using func more and more now that it's available (not only as an alias for proc). I think it's really helpful to know a proc is {.noSideEffects.} (without having to write that pragma all the time :P).

Didn't do it everywhere yet, simply because I forgot, haha.

edit: basically the same reason why I love Nim for preferring let over var.

brentp commented 6 years ago

is this ready for merge?

Vindaar commented 6 years ago

Just let me rebase that on the (now merged) master and then I'd say yes.

Vindaar commented 6 years ago

Done, all good on my end.