[linting] Use eslint to lint code. + minor refactoring + Travis CI setup

[Feder1co5oave]

See the previous discussion at #999.

closes #1014 and closes #1028 .
closes #967

See the commits list for the history of edits.

Apart from some unnoticeable edits, I've changed some minor code style (see var declarations) and refactored the replace() function in a OOP way. I believe it looks neat and self-documenting, I'd like to hear from others on this, and accept different proposals.

With this PR, eslint is completely happy with the source code as it is, and reports no warning or errors.
The programmer can run npm run lint to autofix code style, and get linting feedback before submitting new PRs.

Future steps: linting test/index.js 😄

Thanks a lot to @smhg and @joshbruce for their contributions.

[joshbruce]

I like it! Definitely seems easier to read and makes it more declarative in its intent.

I'm going to flag it for 0.5.0 for now to allow for conversation while still focusing on fixing known issues...does anyone think it should be part of 0.4.0?? (Do we think it will actually help folks fix the defects we've found to date??)

[UziTech]

I think this would help development greatly.

Since this doesn't affect dependents, I see no reason to wait for a release.

@Feder1co5oave I am getting a warning that eslint-plugin-import and eslint-plugin-promise are peer dependencies of eslint-config-standard. Should those be added under devDependencies?

[joshbruce]

@UziTech and @Feder1co5oave: Let me know if I should merge now or wait until the devDependencies thing is figured out. Sorry if I'm out of sync - long day of humaning. :)

[Feder1co5oave]

Let's wait a bit, I want to do a dependency check-up beforehand, and be sure to get dependency versions correct.

[Feder1co5oave]

Can you confirm that using local copies of gulp and eslint is the right way to go? I think it's safe assuming the dependencies are installed locally?
NEVERMIND, I found this. Turns out npm puts node_modules/.bin/ in $PATH when running tasks.

[smhg]

You're absolutely right. Local copies are definitely preferred.
This comment was confusing. When you want to run the ESLint --init script once when only having a local copy, you'll need to add the full node_modules path. But for reusable scripts NPM is great (with its path, among others).

[Feder1co5oave]

Thanks for confirming that. I forgot to push.

[UziTech]

prepack sounds like the right place to put it. That way if someone requires marked as a git url the build will happen before it is downloaded.

[joshbruce]

@UziTech: Agreed. Going to submit a WIP PR to continue conversation there - also trying to learn all the things!

[joshbruce]

This all looks good to me. Would like to get a second pair of eyes though as it looks like we have lint + Travis (woohoo! on both). Want to make sure some of the changes to marked.js are just making things a bit more explicit; the addition/change to edit().replace, for example.

[joshbruce]

Looks like @UziTech will be coming in behind me; so, let me know if this should result in an NPM-publish once merged. Would like to go through that process with CI and whatnot a couple more times before adding more publishers to refine the PR templates and process docs.

[joshbruce]

Thank you @Feder1co5oave. You're awesome!

[UziTech]

we should probably add node to the versions to get the latest stable version (Node 9.x right now)

[Feder1co5oave]

Added. I wanna leave the lts in there too since it is required by the selenium-webdriver which is something I'm working with to do cross-browser tests on Browserstack 😉

[UziTech]

Ya the minimum I always test is lts/* and node

[Feder1co5oave]

https://travis-ci.org/Feder1co5oave/marktex/builds/345826064

(still failing due to that link encoding issue I'm solving)

[Feder1co5oave]

Josh I think you can finally enable Travis on this repo 🥂

[joshbruce]

See #1076

[TheDancingCode]

Is it the aim to eventually remove the rules that override the standard config? I can help with that if you want.

[styfle]

@TheDancingCode

I don’t believe so. I think the idea was to get lint tests that matched the current code style. But we need to figure out our roadmap a little better.

[joshbruce]

@TheDancingCode: To be honest, I'm not sure I'm following (sorry, still figuring out the vernacular of the users). Would you at least be willing to submit an issue with a little exposition on the idea??

[Feder1co5oave]

Yes, the original idea was to progressively adapt the code style to (semi)standard.
However, the only rules I would possibly re-enable are

• space-before-function-paren
• one-var (this is the big one. Var declaration should be revisited all around the source code, see 🚧[Experimenting]: Refactoring marked js🚧 #1102 (comment))

@TheDancingCode
The following rules are disabled because they would not help readability:

• no-cond-assign
• no-useless-escape
• no-return-assign

The following must stay disabled:

• no-control-regex

I don't think it worth working on this (apart the one-var). Consider that re-linting code may involve big patches that will likely bring merge conflicts with PRs that were branched from an earlier commit.