Some ideas and on going development of new plugin called MagicBlock

frissdiegurke

By the way, is nobody interested on this plugin except two, really?

In my opinion it's simply too much. I don't see the use of allowing all those markup. I like the simplicity of markdown and love the boundaries of it (with a few exceptions).
Allowing safe HTML can be nice, but I think most of this community is used to markdown like me

However since there definitely are people who would benefit from your plugin, keep up the great work

Some words about the code style, since you asked:

Since you only have a synchronous procedure, it's all fine using js similar to other languages
Your way of avoiding code tags is quite nice thought it could fail when users type CODE manually, couldn't it?
- I'm using this code to achieve code-block exclusions. It does not handle pre blocks explicitly, but since the markdown standard promises to use a code tag within each pre, it's fine
It is common practice to use CapitalCase naming only for constructor functions, not for important objects. A common shortcut for module.exports is exports = module.exports = {...} and referencing exports later on. exports is also pre-defined like this if you don't overwrite the pre-defined module.exports object.
Since your code is getting quite large, you should consider splitting it into sub-modules (take a brief look at the docs for more info). I'd recommend to use the entry point only for exports needed by the hooks. This is a typical entry-point of mine.
You should always export as few as possible. In your case only the parse function should get exported. Even the options (once implemented) should become a local variable defining default options while one may pass an options object to the parse function.

frissdiegurke

Just two more:

You're using a IIFE. This is not necessary for Node.js modules.
You're using ` for defining strings at some places. You should be aware that this is an ECMAScript2015 (ES6) feature and this is not supported by Node.js 0.12. Nowadays Node.js 0.12 is a bit outdated, nonetheless debian stable still lists 0.10.29. Your choice to (not) support it

If you intend to use ES6 it's common to fully replace var with let (block-scoped variables).
You can also use a tool called babel to transpile your code to ES5 for supporting at least node.js 0.12 (and with high chance 0.10.29, I'm not sure which features of ES5 that does not implement). Combine this with a task-runner (typically grunt or gulp; gulp is more beloved nowadays) and you're fine

qgp9

@frissdiegurke,
Thank you for advises.
You threw a lot which will eat my weekend
I'll follow up your list slowly.

AND

@frissdiegurke said:

In my opinion it's simply too much. I don't see the use of allowing all those markup. I like the simplicity of markdown and love the boundaries of it (with a few exceptions).
Allowing safe HTML can be nice, but I think most of this community is used to markdown like me

Actually, I agree with your opinion and that was my dilemma also.
My needs were just colored text and a controllable inframely. And I'm using my plugin for only them.
simply {{#red TEXT}} and {{ URL }}

But after I implemented those small functions, then I also thought somebody may want more exactly same as what you said in the end.

( Even now I'm back and thinking how admin can limit usages of users to keep clean by length? limit of functional?.. it's so ironic )

exodo

Using :: will break plugin spoiler ?

qgp9

@exodo sorry but I don't catch what you mean plugin spoiler.
Can I have more details?

qgp9

@exodo said:

Using :: will break plugin spoiler ?

OK, I got nodebb-plugin-ns-spoiler

Well I don't think so. I'll explain it with edit after submit this before you answer

frissdiegurke

@exodo nodebb-plugin-ns-spoiler uses :{3,} within the regex' so :: should not break it

qgp9

@frissdiegurke said:

@exodo nodebb-plugin-ns-spoiler uses :{3,} within the regex' so :: should not break it

I am already late!!

Let me explain even more.
Every syntax of MagicBlock include :: works only in the block {{ .. }} so it will not effect even some other plugin which use ::.
More over, which means that MagicBlock will not effect any other plugin which doesn't use {{ or }} except when user intend it.

Please let me know if I missed any other case.

But, other plugins can make effects to MagicBlock..
Yes, I need to consider that case.

Jared14406

Cool (Need more than 8 characters)

qgp9

@frissdiegurke

I have followed up your comments except only two.
https://github.com/qgp9/nodebb-plugin-magicblock/issues/1
It seems that I'm a quite good student, doesn't it?

@frissdiegurke said:

Since your code is getting quite large, you should consider splitting it into sub-modules (take a brief look at the docs for more info). I'd recommend to use the entry point only for exports needed by the hooks. This is a typical entry-point of mine.

This, I have to take, but I may need some time.

@frissdiegurke said:

Your way of avoiding code tags is quite nice thought it could fail when users type CODE manually, couldn't it?

I'm using this code to achieve code-block exclusions. It does not handle pre blocks explicitly, but since the markdown standard promises to use a code tag within each pre, it's fine

If I understood your code correctly, it deals/parses each part of before/between/after code blocks separately. Am I right?
I like this idea, but in my case, I want to handle code block in a MagicBlock something like {{#red some `codeblock` is here}} and the splitting approach will not work.
I may be able to solve the problem with a random string instead of just ___CODE___ and check if that string is in contents before parsing even though it consume additional time.

frissdiegurke

@qgp9 said:

@frissdiegurke

I have followed up your comments except only two.
https://github.com/qgp9/nodebb-plugin-magicblock/issues/1
It seems that I'm a quite good student, doesn't it?

It does

@qgp9 said:

@frissdiegurke said:

Your way of avoiding code tags is quite nice thought it could fail when users type CODE manually, couldn't it?

I'm using this code to achieve code-block exclusions. It does not handle pre blocks explicitly, but since the markdown standard promises to use a code tag within each pre, it's fine

If I understood your code correctly, it deals/parses each part of before/between/after code blocks separately. Am I right?
I like this idea, but in my case, I want to handle code block in a MagicBlock something like {{#red some `codeblock` is here}} and the splitting approach will not work.
I may be able to solve the problem with a random string instead of just ___CODE___ and check if that string is in contents before parsing even though it consume additional time.

You're right, it does work that way.
I have a better idea: Since your script gets executed (you should add a priority option to the parse hooks btw. to ensure this) after the markdown parser, you can use a string that may never pass the markdown parser, e.g. <<code>> since < gets passed as < except for real HTML-tags for sure.

qgp9

@frissdiegurke said:

I have a better idea: Since your script gets executed (you should add a priority option to the parse hooks btw. to ensure this) after the markdown parser, you can use a string that may never pass the markdown parser, e.g. <<code>> since < gets passed as < except for real HTML-tags for sure.

You are just a genius.

frissdiegurke

I also think the code quality suffers a lot from inline functions. I've trained me to use global functions (even below all exports) and reference them where they need to be exported.

I try to keep my files like this (it's just about structure; keeping ' instead of " and dropping ; is up to you):

"use strict";

// define dependencies to other modules
let x = require("x");

// and local ones separated by one empty line
let y = require("./y");

// constants
const XYZ = "XYZ";

// "global" variables (as few as possible)
let myVar = null;

/*======================== Exports  ========================*/

// exports (variables first)
exports.lastText = null;
// if possible demonstrate the variable type, even if it gets overwritten without being read once
exports.options = {};

// exports (functions last)
exports.parse = parse;
exports.init = function (cb) { cb(); } // simple one-liner are ok in my opinion

/*==================== Initial Workflow ====================*/

// procedure on module-load (if any, as few as possible); if exceeds ~5 lines, it should become a function
console.log(parse(XYZ));

// just for demonstration purpose
return; // The function definitions are independent of procedural workflow.

/*======================= Functions  =======================*/

function someUtil(text) { exports.lastText = text; } // one-liner are ok in my opinion

function parse(text) {
  someUtil(text);
  return text.replace(/X/g, function () { return "<<X>>"; });
}

This way there is no module whose dependencies and exports I cannot see on the top of the file (and thus on first page of screen in most cases).
The exports are primarily sorted as stated above (variables/functions), secondarily by logical connection.
The order of the function definitions might as well be sorted by logical connection but is basically irrelevant since you can jump to ones definition with most IDEs via the reference within the exports section.
Following this structure you also don't need to reference exports within your functions except you want to modify/read variables within. Function calls go without exports access.

Except the separation lines (/*=== ... ===*/) I don't recommend the usage of any comments. Documentation is great of course (but as long you keep the functions short it's not required)

This is just a recommendation of what I've learned to use within the last year (in retrospective my code was not quite readable before)

And of course I'd also be thankful for any kind of code quality recommendations of other people
Feel free to criticize my schema.

qgp9

@frissdiegurke

An upvote is not just enough!

I have been trying to catch your style and realize that it's quite reasonable and clean ( but I need more practical understanding , maybe by reading of your plugin and module. )

I really think that we need to elevate this coding style and policy at some point. I read several plugins and some of them seem to need to be guided like mine. ( I don't blame those plugins, one of my motto is that "something is not better than nothing ( of course not always ) )

Jared14406

@qgp9 lol I need 3 upvoted it's good to have upvotes it gives u reputation!!!!