-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fixed example codes (syntax) #3730
Conversation
# | ||
# Levenshtein.distance("algorithm", "altruistic") # => 6 | ||
# Levenshtein.distance("hello", "hallo") # => 1 | ||
# Levenshtein.distance("こんにちは", "こんちは") # => 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indent
# NamedTuple(foo: String, bar: Int64).from({:foo => "world", :bar => 2}).class # => {foo: String, bar: Int64} | ||
# NamedTuple(foo: String, bar: Int32).from({:foo => "world", :bar => 2}) # => {foo: "world", bar: 2} | ||
# NamedTuple(foo: String, bar: Int32).from({"foo" => "world", "bar" => 2}) # => {foo: "world", bar: 2} | ||
# NamedTuple(foo: String, bar: Int32).from({:foo => "world", :bar => 2}).class # => {foo: String, bar: Int32} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing Int64
to Int32
for bar
disables casting feature which this example is about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, the casting feature is already broken in 0.20.1.
So I assumed the spec has been changed.
I'll revert it soon. Then, would I create the issue?
We could probably have a way to let the doc generator compile and run these small examples, even checking if they return the correct values. So something similar to |
d440345
to
d502626
Compare
# | ||
# Levenshtein.distance("algorithm", "altruistic") # => 6 | ||
# Levenshtein.distance("hello", "hallo") # => 1 | ||
# Levenshtein.distance("こんにちは", "こんちは") # => 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still wrong indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sija I'm pretty sure this might end up being screwy no matter what; the Unicode symbols used are variable-width, so different fonts are all going to show it a bit differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kirbyfan64 hmm, maybe it's a formatter bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be a formatter bug. I'll fix it soon :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it's not a bug. Japanese characters have a longer width, so it pushes the comment to the right...
d502626
to
71c0d3d
Compare
@asterite Having examples compile by default is really nice, because it opens the path for a possible future PR making doc examples interactive in the browser. Will probably have to wait until we get a wasm or JS compiler target however. Unfortunately having every example compile would reduce brevity, so there should probably be an option to turn this off example-by-example. Or maybe there could be a way to hide parts of the example so you could create examples which compile, but only show the non-boilerplate code. In addition, I would make testing the doc examples part of the spec runner, because having a piece of code and an expected result makes it a specification. However, if you want to keep it part of |
@RX14 I'm trying to compile every code example right now, and it's taking forever. I agree with all of your points and I have the same concerns too. I think it will probably remain as an idea, at least for some time :-) |
71c0d3d
to
47a9227
Compare
Forgot to run |
@asterite one thing we could do now is add an option to dump the example code to a directory. This would make extracting the example code when we do want to manually check it easier. |
@maiha Thank you so much for this! Keeping example code up to date and compiling is super important! |
It seems that some of commented codes are wrong or out of date.
I tried to compile all 1088 examples, and fixed syntax failures as possible.
master
PR
There are still failures where the problem is semantics level such as
I'll create another PR about this when I have a chance.
Thanks.