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

Commented out values get eaten #5

Open
yminsky opened this issue Aug 14, 2017 · 10 comments
Open

Commented out values get eaten #5

yminsky opened this issue Aug 14, 2017 · 10 comments

Comments

@yminsky
Copy link
Member

yminsky commented Aug 14, 2017

I just noticed this in the Async chapter, where some commented out code got removed entirely in the corrected file. I think the corrected file should never remove comments.

@let-def
Copy link
Collaborator

let-def commented Aug 16, 2017

Can you give one example?

@yminsky
Copy link
Member Author

yminsky commented Aug 21, 2017

Hmm. Can't seem to replicate. I'm not sure what I was seeing, but will re-open if/when I experience it.

@yminsky yminsky closed this as completed Aug 21, 2017
@yminsky
Copy link
Member Author

yminsky commented Aug 21, 2017

Just got it again. When I comment out this section:

[%%expect{|an error happened|};
ocaml {|- : unit = ()|}];;

[@@@part "36"];;
(*
exception Ignore_me;;
[%%expect ocaml {|exception Ignore_me|}];;
let swallow_some_errors exn_to_raise =
  let child_monitor  = Monitor.create  () in
  let parent_monitor = Monitor.current () in
  Stream.iter
    (Monitor.detach_and_get_error_stream child_monitor)
    ~f:(fun error ->
      match Monitor.extract_exn error with
      | Ignore_me -> printf "ignoring exn\n"
      | _ -> Monitor.send_exn parent_monitor error);
  within' ~monitor:child_monitor (fun () ->
    after (Time.Span.of_sec 0.25)
    >>= fun () -> raise exn_to_raise)
;;
[%%expect ocaml {|val swallow_some_errors : exn -> 'a Conduit_async.io = <fun>|}];;
*)

[@@@part "37"];;
Deferred.any [ after (Time.Span.of_sec 0.5)
             ; swallow_some_errors Not_found ]
;;
[%%expect{|Exception: (monitor.ml.Error Not_found ("Caught by monitor (id 61)")).|}];;
[@@@part "38"];;
Deferred.any [ after (Time.Span.of_sec 0.5)
             ; swallow_some_errors Ignore_me ]
;;

in the concurrent programming section, then topscript wants to delete it.

@yminsky yminsky reopened this Aug 21, 2017
@yminsky
Copy link
Member Author

yminsky commented Aug 21, 2017

And this shows the state of the code and the result of the diff against the corrected file side-by-side. The red shows that the commented out bit is deleted.

image

@yminsky
Copy link
Member Author

yminsky commented Oct 4, 2017

Did we ever get a resolution on this?

@let-def
Copy link
Collaborator

let-def commented Oct 4, 2017

It shouldn't occur with master. Is it still happening?

@yminsky
Copy link
Member Author

yminsky commented Oct 4, 2017 via email

@let-def
Copy link
Collaborator

let-def commented Oct 4, 2017

No it is not released yet.

@yminsky
Copy link
Member Author

yminsky commented Oct 4, 2017 via email

@let-def
Copy link
Collaborator

let-def commented Oct 4, 2017

Sure.

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

No branches or pull requests

2 participants