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

Add html-elements tool and docs for other tools/ #493

Merged
merged 2 commits into from
Dec 8, 2019

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Dec 7, 2019

Type of Change

Enhancement

Description

This adds a new html-elements tool, which allows us to get info about all the HTML elements used in blog posts, and their counts. I wanted to know what people actually use, so we can build our UI and other tools to accommodate it.

You run it like this:

$ npm link
$ html-elements
Processing 5534 posts...
Processing 194,922 elements...
1. <br> (69,015)
2. <p> (36,408)
3. <a> (19,760)
4. <div> (15,244)
5. <li> (8,779)
6. <td> (7,124)
7. <strong> (6,432)
8. <img> (6,205)
9. <code> (4,448)
10. <pre> (4,076)
11. <b> (3,176)
12. <em> (2,699)
13. <ul> (2,325)
14. <tr> (1,790)
15. <h3> (1,726)
16. <i> (1,380)
17. <h4> (1,075)
18. <blockquote> (1,004)
19. <ol> (538)
20. <hr> (441)
21. <table> (365)
22. <tbody> (364)
23. <th> (269)
24. <h5> (133)
25. <iframe> (63)
26. <strike> (37)
27. <h6> (27)
28. <thead> (14)
29. <caption> (5)

I've also added docs for our tools/ dir, so people can figure out what things do.

My code was done in such a way that we can write other analysis tools to iterate over posts easily.

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionalty or configuration variables are added/changed or an explanation of why it does not(if applicable)

Copy link
Member

@manekenpix manekenpix left a comment

Choose a reason for hiding this comment

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

Tested it on my machine using your branch.
Screenshot from 2019-12-07 18-45-58

It failed when I tried npm link though, I had to run it with sudo:
Screenshot from 2019-12-07 18-46-44

@humphd
Copy link
Contributor Author

humphd commented Dec 8, 2019

@manekenpix
Copy link
Member

manekenpix commented Dec 8, 2019

@humphd normally I don't have to use sudo for anything when using npm.
I tried sudo chown -R but it didn't solve the issue (the folder is already mine).

Someone mentions this in the link:

Permissions you used when installing Node will be required when doing things like writing in your npm directory (npm link, npm install -g, etc.).
You probably ran node installation with root permissions, that's why the global package installation is asking you to be root.

I used my distro's package manager to install npm so I guess technically I did install it with root permissions. Maybe you could include this in the README to solve this problem?

@humphd
Copy link
Contributor Author

humphd commented Dec 8, 2019

@manekenpix good idea, I've pushed another commit with a note.

Two things:

  1. Have you ever used nvm to manage your node versions? I'd highly recommend it over your distro's or package manager's, because you can easily switch between different versions without breaking things (good for testing, working on something with a fixed version requirement, etc).
  2. I notice that my PR built on Circle CI just now, cc @sukhbeersingh.

@manekenpix
Copy link
Member

@humphd I used nvm once in class when we cloned VSCode and tried to build it. I think I should start using it on all my machines. Thanks for letting me know.

@manekenpix manekenpix self-requested a review December 8, 2019 16:14
@humphd humphd merged commit ccdb3e9 into Seneca-CDOT:master Dec 8, 2019
@humphd humphd deleted the html-elements branch December 8, 2019 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants