Closed bilkusg closed 6 years ago
Ok I took the time to read your code, and I don't think that using the Utils.c
function is a good way to comment the code.
Problem is the output is really compact (I tried to fix that but wasn't feeling that nice), lack color, etc.
I personnaly would prefer, if the F# code is commented the normal way. Because, there we can indent the code, have synthax coloration support, tooltip, etc.
Showing the console ouput in the preview page, is nice if we only output minimal info.
For example, I would rewrite this lines:
c "START HERE"
c "Manipulating a value which is really a pojo object under the hood"
c "Add a new value to an object"
c "boringObj.newValue = \"newValue\""
boring1Obj?newValue <- "newValue"
Browser.console.log("boring = ",boring1Obj)
as
// Manipulating a value which is really a pojo object under the hood
// Add a new value to an object
// boringObj.newValue = "newValue"
boring1Obj?newValue <- "newValue"
Utils.code "boring1Obj?newValue <- \"newValue\"" boring1Obj
with:
module Utils =
open Fable.Core
open Fable.Import
[<Emit("JSON.stringify($0, null, 4) + ''")>]
let anyToString (_: obj) : string= jsNative
let code codeString value =
Browser.console.log("> " + codeString + ":")
Browser.console.log(anyToString value)
This result in:
The content is really interesting 👍 , we need to work on the visual aspect :)
Hi, Thanks for your comments and kind words on the substance. I will work on incorporating the requested updates and let you know when I've changed it.
Just one point - I can't find a way to move the console output div below the text rather than above it, as you suggest. This is because the javascript to copy the console can't see inside the iframe so it creates a new window above it..... Hope that's OK.
I'm about half way through changing the notation style now.
OK I have updated the repo and pull request with the requested changes. Let me know if this is now what you want.
@alfonsogarciacaro Can I let you review the changes and decide to merge or no the PR.
I made a pass on the code to made it cleaner. Still not sure of the result.
@bilkusg @MangelMaxime Sorry, I thought you were dealing with this and didn't check the last notification 😅 I just left a couple of minor comments but this is OK for me to merge :+1:
Is there anything else needed before this can go live? I'm hoping to refer to the repl in some articles I'm writing, but I obviously need to wait until the merge happens.
Thanks,
@bilkusg Just need to change the code according to Alfonso comments and then we are ready to merge.
Not for myself, make sure to use Fable 1.3.7 before depoying new version because @ncave suppose there is a bug in the REPL with newer version of Fable .
I'd say let's merge this. Comments are not so important anyways :smile: I'll push the sample :+1:
Ok thanks @alfonsogarciacaro
…ntation