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

Comment Parser #12

Open
3 tasks
Glavin001 opened this issue Feb 18, 2014 · 4 comments
Open
3 tasks

Comment Parser #12

Glavin001 opened this issue Feb 18, 2014 · 4 comments

Comments

@Glavin001
Copy link
Contributor

TODO:

  • Switch to using https://github.com/sindresorhus/get-urls for parsing URLs from the comment string
  • Include the GitHub username in Comment object
  • Include date comment added (also date updated) in Comment object
@Glavin001
Copy link
Contributor Author

I do not think that the comment_parser should deal with the DOM as it does currently with it's htmlList element.

  • Separate UI updating from the comments_parser

@vasilionjea
Copy link
Contributor

@Glavin001
Copy link
Contributor Author

  • Bug: It should be url.indexOf(domains) > -1 instead of url.indexOf(domains) > 1.

@vasilionjea after PR #13 is merged, would you be able to assist me in breaking up the comments_parser?

I am thinking:

Player

  • SoundManager player, see Main Music Player #6
  • playTrack(track) - Given a Track, detect if it is a supported URL (SoundCloud, etc) and then use the respective SDK to retrieve a SoundManager sound object. Finally, sound.player().

Tracks (originally comment_parser)

  • Comments are parsed into one or many Tracks, given a single or multiple URLs.

What should we include in the Track object?
Potentially:

{
"user": "GitHub username of the user who created the Comment.",
"url": "The URL as parsed from the original Comment.",
"dateAdded": "Date the Comment was added.",
"comment": "Should we store a reference to the original Comment?"
}

What are your thoughts?

@vasilionjea
Copy link
Contributor

As far as removing the htmlList, yes I agree that it has nothing to do with parsing comments and should be done on the outside.

Now on CommentsParser itself, I'm not sure what else we could separate. From your comment above I think you want to rename CommentsParser to Track or I think you're saying you want CommentsParser to instantiate new Track objects once the Track constructor exists with properties like the JSON above in your comment.

Something to keep in mind in general is that not all the URLs will be turned into Tracks because most likely we won't integrate with all the APIs from all sources. The URLs that won't become Tracks should instead be provided in another tab as anchor links. Basically this was the idea behind returning a result object with hashed links: https://github.com/Glavin001/TasteMusic/blob/master/app/scripts/comments_parser.js#L13

The other key in the result hash would be for the anchor links.

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