Skip to content

Conversation

@Volte6
Copy link
Member

@Volte6 Volte6 commented May 1, 2025

Description

This adds a makefile target to run a check on js files through jshint:

make js-lint

Experimented with a couple other tools such as eslint but they proved cumbersome and overly strict, requiring annotations in script files to direct behavior more and more. Things like spacing, undefined functions (because they were defined in a .go binding) etc. were problematic.

Settled on jshint since it's looser in its rules. Primarily we are testing for:

  • syntax errors that will break execution
  • Incompatible keywords
  • "good" practices such as semicolons at the end of lines.

There is some filtering out of noise from the docker image (npm error, npm warn, etc) that can be ignored. Otherwise, exits with code 1 if errors are detected, and reports errors.

This should allow us to implement in CI/CD processes.

Changes

  • js-lint make target.
  • Handled existing js errors reported (a lot).

Example Output

With errors:

√ ~/dev/GoMud % make js-lint
_datafiles/sample-scripts/spells/harmmulti.js: line 10, col 27, Expected ')' and instead saw 'eActor'.
_datafiles/sample-scripts/spells/harmmulti.js: line 10, col 44, Expected an assignment or function call and instead saw an expression.
_datafiles/sample-scripts/spells/harmmulti.js: line 10, col 72, Missing semicolon.
_datafiles/sample-scripts/spells/harmmulti.js: line 10, col 72, Expected an identifier and instead saw ')'.
_datafiles/sample-scripts/spells/harmmulti.js: line 10, col 72, Expected an assignment or function call and instead saw an expression.

5 errors
make: *** [js-lint] Error 1
√ ~/dev/GoMud %   

Without errors:

√ ~/dev/GoMud % make js-lint
√ ~/dev/GoMud %   

Links

#342

@Volte6 Volte6 marked this pull request as ready for review May 1, 2025 17:37
@Volte6 Volte6 merged commit 3b44018 into master May 1, 2025
2 checks passed
@Volte6 Volte6 deleted the jshint branch May 1, 2025 18:20
@colinsenner
Copy link
Contributor

Do you want this run on commits/prs in ci/CD? @Volte6

@Volte6
Copy link
Member Author

Volte6 commented May 1, 2025

Do you want this run on commits/prs in ci/CD? @Volte6

I think that's the goal, just not in this PR

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.

4 participants