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.

Sign up for GitHub

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

Add jsdomparser #13920

Merged
merged 10 commits into from
Apr 13, 2020
Merged

Add jsdomparser #13920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
48f471e
Add jsdomparser
juancarlospaco Apr 8, 2020
87f8738
Add jsdomparser
juancarlospaco Apr 8, 2020
6f89c3d
Add jsdomparser
juancarlospaco Apr 8, 2020
d381256
https://github.com/nim-lang/Nim/pull/13920#issuecomment-610727142
juancarlospaco Apr 8, 2020
ea65e27
https://github.com/nim-lang/Nim/pull/13920#issuecomment-610727142
juancarlospaco Apr 8, 2020
bb04479
https://github.com/nim-lang/Nim/pull/13920#issuecomment-610727142
juancarlospaco Apr 8, 2020
4f3e290
https://github.com/nim-lang/Nim/pull/13920#discussion_r405932909
juancarlospaco Apr 9, 2020
6172d73
Merge branch 'devel' of https://github.com/nim-lang/Nim into jsdomparser
juancarlospaco Apr 9, 2020
92f5394
Merge branch 'devel' of https://github.com/nim-lang/Nim into jsdomparser
juancarlospaco Apr 10, 2020
2c2739e
https://github.com/nim-lang/Nim/pull/13920#discussion_r406502592
juancarlospaco Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

## Standard library additions and changes

- Add [DOM Parser](https://developer.mozilla.org/en-US/docs/Web/API/DOMParser)
to the `dom` module for the JavaScript target.

## Language changes

Expand Down
19 changes: 18 additions & 1 deletion lib/js/dom.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

## Declaration of the Document Object Model for the `JavaScript backend
## <backends.html#backends-the-javascript-target>`_.

include "system/inclrtl"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this being added?

Copy link
Contributor

Choose a reason for hiding this comment

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

Its not needed and should be removed

Copy link
Member

Choose a reason for hiding this comment

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

since

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

since

Copy link
Contributor

Choose a reason for hiding this comment

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

I know it includes since but you shouldn't have to manually include it.

Copy link
Member

Choose a reason for hiding this comment

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

you need to otherwise you get CT error; but if we merged #11865 we could import it instead of including it

Copy link
Contributor

Choose a reason for hiding this comment

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

Once again putting it out there that I consider this since thing to be a waste of time.

when not defined(js) and not defined(Nimdoc):
{.error: "This module only works on the JavaScript platform".}

Expand Down Expand Up @@ -985,6 +985,16 @@ type
once*: bool
passive*: bool

since (1, 3):
type DomParser* = ref object ## \
## DOM Parser object (defined on browser only, may not be on NodeJS).
## * https://developer.mozilla.org/en-US/docs/Web/API/DOMParser
##
## .. code-block:: nim
## let prsr = newDomParser()
## discard prsr.parseFromString("<html><marquee>Hello World</marquee></html>".cstring, "text/html".cstring)


proc id*(n: Node): cstring {.importcpp: "#.id", nodecl.}
proc `id=`*(n: Node; x: cstring) {.importcpp: "#.id = #", nodecl.}
proc class*(n: Node): cstring {.importcpp: "#.className", nodecl.}
Expand Down Expand Up @@ -1297,3 +1307,10 @@ proc offsetHeight*(e: Node): int {.importcpp: "#.offsetHeight", nodecl.}
proc offsetWidth*(e: Node): int {.importcpp: "#.offsetWidth", nodecl.}
proc offsetTop*(e: Node): int {.importcpp: "#.offsetTop", nodecl.}
proc offsetLeft*(e: Node): int {.importcpp: "#.offsetLeft", nodecl.}

since (1, 3):
func newDomParser*(): DOMParser {.importcpp: "(new DOMParser()​​)".}
## DOM Parser constructor.

func parseFromString*(this: DOMParser; str: cstring; mimeType: cstring): Document {.importcpp.}
Copy link
Member

@timotheecour timotheecour Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

still meh on that; at least provide an enum to make life easier for users eg:

type MimeTypeDom = enum
  textHtml = "text/html" etc

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't. JS grows relatively fast and change really quickly, so we should not spent too much time chasing these things. Different browsers might support more MIME types, and it's not really our place to dictate how to use an API.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with alaviss on this point.

Copy link
Member

Choose a reason for hiding this comment

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

fine, we can later introduce

type MimeTypes = enum
  textHtml = "text/html".cstring
  # etc

and use it as:
let a = prsr.parseFromString("<html>...</html>".cstring, $textHtml)

(but not sure what's the difference with fetch API case https://github.com/nim-lang/Nim/pull/12531/files#r403861811

So what, please use enums.
)

## Parse from string to `Document`.