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

Convert Spotify Package to JS #6449

Merged
merged 3 commits into from
Mar 23, 2017
Merged

Convert Spotify Package to JS #6449

merged 3 commits into from
Mar 23, 2017

Conversation

MartinSchoeler
Copy link
Contributor

@RocketChat/core

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6449 March 23, 2017 18:20 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-6449 March 23, 2017 18:39 Inactive
Template.registerHelper('match', (source, regex) => new RegExp(regex).test(source));

Template.oembedBaseWidget.onCreated(function() {
if (this.data && (this.data.meta && /(music.song|music.album)/.test(this.data.meta.ogType)) && this.data.parsedUrl && this.data.parsedUrl.host === 'open.spotify.com') {
Copy link
Member

Choose a reason for hiding this comment

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

The correct regex should be /^(music\.song|music\.album)$/

Copy link
Member

Choose a reason for hiding this comment

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

Or you can use ['music.song', 'music.album'].includes(this.data.meta.ogType)

// Verify if this part is code
const part = msgParts[index];
let item;
if (((part != null ? part.length : undefined) != null) > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

That comparison will not work, will be true > 0 or false > 0

let item;
if (((part != null ? part.length : undefined) != null) > 0) {
const codeMatch = part.match(/(?:```(\w*)[\n ]?([\s\S]*?)```+?)|(?:`(?:[^`]+)`)/);
if ((codeMatch == null)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why double parenthesis?

const msgParts = source.split(/(```\w*[\n ]?[\s\S]*?```+?)|(`(?:[^`]+)`)/);

return (() => {
const result = [];
Copy link
Member

Choose a reason for hiding this comment

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

You can remote this return and result.
Coffee always return the last line, but we are not using the return of this method here


process(message, message.msg, function(message, msgParts, index, part) {
const re = /(?:^|\s)spotify:([^:\s]+):([^:\s]+)(?::([^:\s]+))?(?::(\S+))?(?:\s|$)/g;
return (() => {
Copy link
Member

Choose a reason for hiding this comment

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

No need for this return too

let process = undefined;
class Spotify {
static initClass() {
process = function(message, source, callback) {
Copy link
Member

Choose a reason for hiding this comment

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

You can move this process to outside the classe

@rodrigok rodrigok merged commit 461b7bc into develop Mar 23, 2017
@rodrigok rodrigok deleted the spotify-to-js branch March 23, 2017 20:31
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

Successfully merging this pull request may close these issues.

3 participants