search

taoqf / node-html-parser

A very fast HTML parser, generating a simplified DOM, with basic element query support.
MIT License
1.12k stars 112 forks source link

[Feature] Add getElementById() #165

Closed Pomax closed 2 years ago

Pomax commented 3 years ago

Given that there's support for getElementsByTagName, it would be nice if there was support for getElementById, too.

The main benefit would be that there are valid id attributes that you can't query-select, like id attribute with periods, which will not work as query selection string because periods will be treated as class markers. E.g. <section id="attr.name"> cannot be query-selected using #attr.name as that will instead look for an element with id="attr" and class="name".

nonara commented 3 years ago

PR welcome! It should conform to MDN spec and have tests.

Pomax commented 3 years ago

I'm sure they are, but if I had time to dive into someone else's codebase I would already have a PR for you, tied to this issue. Instead, this is a feature request that I definitely won't have time to write for you.

nonara commented 3 years ago

First, that message was for you or anyone reading this thread who wants to contribute.

Second, this isn't my code base. Like you, I was a user. I also did not have time, but I submitted a few fixes and ultimately decided to help people out because I saw the backlog was pretty bad. I work full time and absolutely should not be doing OSS work, but I choose to in order to help out how I can. To that end, I asked the maintainer of this library a week ago and he gave me access.

Third, you requested the feature. You would be writing it for you and for anyone else who wants it. I personally have no need for it, so it would not be for me. (Though I agree the pairing makes sense - more on that below)

Finally, posting "PR welcome" does not mean I will never address it. It means that if someone else would like to, they can do so. Otherwise, because I agree that it would be good to have this in addition to the other feature, I most likely will add it during the next batch, when I find time to work on it.

That said, I also did not add getElementByName. It was a PR from someone who posted an issue asking for it. I responded "PR is welcome", and he added it. He did a great job.

This is free, open source software. If you dont have time, believe me, the maintainers likely have less. This is a labour of love for a largely rude and thankless audience, and it is attitudes like this that make us routinely question our own sanity in donating our time toward it.

Pomax commented 3 years ago

I know that's not what you mean, but just a "PRs welcome" reply to FOSS issues has become a bit of a negative thing to say in the last few years, as people have more and more started taking it to mean "you get to add it yourself, we don't care". If that's not what you meant: great! But then being a bit longer than two words would be good idea too, where you make it explicit that it's a general for-everyone comment, with links to "new dev" docs that folks can use to read into the codebase etc. and possibly with the "help wanted" label added so that people searching github for projects that would appreciate some help, can find those issues.

A great github-built-in tool here is the template replies option for comments, where you can write that welcome message once, and then you can just insta-add it to any issue where it's appropriate. You should be able to see the option for that in the upper right of the comment "box", the left-curved arrow with the pulldown icon next to it.

taoqf commented 2 years ago

Added. v5.3.0. Thank all of you. Sorry for that I didn't added the method earlier. @nonara has already helped a lot.