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

[Docs] Added {.experimental: "codeReordering".} to manual #8806

Merged
merged 5 commits into from
Sep 14, 2018

Conversation

awr1
Copy link
Contributor

@awr1 awr1 commented Aug 29, 2018

Closes #8699.
Might also for #8822. (depends on if {.reorder.} needs new impl)

@awr1
Copy link
Contributor Author

awr1 commented Aug 29, 2018

Something to add wrt declared:

{.reorder: on.}

proc x() =
  echo(declared(foo))

var foo = 4

x() # "false", the same as it would be with {.reorder: off.}

Should this be defined behavior (in which case I will clarify in the docs) or is this an issue?

@Araq
Copy link
Member

Araq commented Aug 30, 2018

Should this be defined behavior (in which case I will clarify in the docs) or is this an issue?

The last time I thought about it, it seems to break less code the way it's handled now.

@Araq
Copy link
Member

Araq commented Aug 30, 2018

Ok, but shouldn't this become {.experimental: "reorder".} before we document it?

doc/manual.rst Outdated
``nim check`` provides this option as well.

Example:
NOTE: The following was documentation for the precursor to {.reorder.},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just remove this? It's just a distraction to keep it there I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hidden at the moment, right? I'm still in favor of noforward as a more comprehensive solution and I hope to work on it in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's commented out in case it becomes relevant again. (if you really really want me to I'll remove it)

@awr1 awr1 changed the title [Docs] Added {.reorder.} to manual [Docs] Added {.experimental: "codeReordering".} to manual Sep 7, 2018
@awr1
Copy link
Contributor Author

awr1 commented Sep 7, 2018

Changed the stuff to {.experimental: "codeReordering".} and moved everything to the scope section (if you think there's a better place to put it, let me know. Also commented out the declared() section since the way it works right now is not entirely obvious. It'd be better to just have the get this all in the docs as an experimental feature at least then decide how to deal with that later.

If we're going with #8875, please merge that before/same time as this.

@alaviss alaviss mentioned this pull request Sep 11, 2018
doc/manual.rst Outdated Show resolved Hide resolved
@Araq Araq merged commit 49d5fb6 into nim-lang:devel Sep 14, 2018
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.

4 participants