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

Add Alan IF Language #1851

Closed
wants to merge 8 commits into from
Closed

Add Alan IF Language #1851

wants to merge 8 commits into from

Conversation

tajmone
Copy link
Contributor

@tajmone tajmone commented Sep 30, 2018

Alan v1.0.0.
Syntax definition for Alan Interactive Fiction language (www.alanif.se).
All Mocha tests passed.

@marcoscaceres
Copy link
Contributor

@tajmone, would you be willing to maintain the Alan IF language? If so, I can spin up a repository for you in highlightjs organization.

@tajmone
Copy link
Contributor Author

tajmone commented Oct 8, 2018

@tajmone, would you be willing to maintain the Alan IF language?

Yes I will, I'm a member of the Alan-IF group and the maintainer of the documentation, so the syntax highlighter definitions (other tools for PDF too) will need to be kept up to date with the language.

If so, I can spin up a repository for you in highlightjs organization.

Will I need to create a new fork for that? My current fork contains some extra branches for non-official mods of the PureBasic language, plus a branch for Alan.

@marcoscaceres
Copy link
Contributor

Up to you. I can spin up a fresh repo or you can transfer yours over.

@marcoscaceres
Copy link
Contributor

actually, from the pull request, we probably only want to take the language definition and tests into the new repository. Be sure not to include hljs itself.

@tajmone
Copy link
Contributor Author

tajmone commented Oct 8, 2018

I was looking at your highlightjs/highlightjs-cshtml-razor as a reference for naming the repository, so I think that it might be better to create a new repository dedicated to Alan specifically. E.g. highlightjs/highlightjs-alan.

actually, from the pull request, we probably only want to take the language definition and tests into the new repository. Be sure not to include hljs itself.

The repository should not be a full clone of hljs? just the syntax definition and tests?

It's not clear to me how this system works, I tried to look for examples but highlightjs-cshtml-razor is currently empty apart from the .gitingore file. How does this workflow work exactly? I'll still need a full clone of hljs locally to work on the syntaxes, how can I make PRs for new changes if the dedicated repo is only partial?

I'm interested in participating, as I've also contributed the PureBasic syntax, which is due for some updates very soon (new Beta version of the language introduces new keywords, just waiting for it to become stable). Also, I've created quite a few syntaxes for other highlighters (non browser) which I was planning to port to hljs too.

The idea of creating a dedicated repo for each syntax makes sense, as it could also be mentioned in the syntax source's comments, and look elegant too. But having to clone the full hljs repo for each syntax might be a bit of an overkill, so I guess that this system you're proposing addresses this issue by keeping it down to the essential files relevant to the syntax at hand.

@marcoscaceres
Copy link
Contributor

The repository should not be a full clone of hljs? just the syntax definition and tests?

Yes, just the syntax definition and tests.

How does this workflow work exactly?

We are trying to work that out - and could use your help. Basically, I would assume usage would be, for tests:

import hljs from 'highlight.js/lib/highlight';
import alan from './lang/alan';
hljs.registerLanguage('alan', alan);

And you would have a package.json that has "highlight.js" as a developer dependency only.

In practice, for every day users, the would then do the same as above too... So, just:

  1. npm install highlightjs
  2. npm install highlightjs-alan
  3. Write code as above (and webpack/babel or node does the right thing).

what do you think?

@tajmone
Copy link
Contributor Author

tajmone commented Oct 15, 2018

I think that it's a good idea, this separation makes it cleaner work on and maintain syntaxes, providing an isolated environment of sorts .

But it shouldn't add a burden on the developer side (e.g. having to manually move files and folders from the dev branch to new repo); so it would be good to have some extra helpers tools/scripts in HJS' repo to help publishing (i.e. detaching) a syntax and it's related files once it's ready.

We are trying to work that out - and could use your help.

I'm not an expert in JavaScript, I only use it occasionally when I have to tweak code, so my knowledge of JS and Node is quite superficial.

I think that the ideal solution would be to either provide a template repository, which can be cloned and used as a sample to create a new syntax repository, or (better) a Node command to create a scaffold repository (maybe with some questions asked, like name of the syntax, etc., to automagically rename folders and fill-in some settings). The latter could either be a tool inside an HJS subfolder, or a proper NPM package that one can install separately.

Probably the advantage of a subfoldered tool is that it will always match the version of HJS it was written for, while an NPM package might outdate the local HJS version if the user doesn't manually pull the latest version.

I noticed that the Razor CSHTML is no longer empty, so I cold look at that as an example to start with.

@marcoscaceres
Copy link
Contributor

But it shouldn't add a burden on the developer side (e.g. having to manually move files and folders from the dev branch to new repo); so it would be good to have some extra helpers tools/scripts in HJS' repo to help publishing (i.e. detaching) a syntax and it's related files once it's ready.

Yeah, I agree... this is an unfortunate issue affecting the whole JS ecosystem - and definitely not specific to this project. Why there is such a reliance on build tools in JS.

For example, I have to do this in my own project I've had to roll my own:
https://github.com/w3c/respec/blob/develop/tools/build-components.js#L22

I'm not an expert in JavaScript, I only use it occasionally when I have to tweak code, so my knowledge of JS and Node is quite superficial.

That's ok. I know JS fairly well so can help provide guidance. I'm also interested to see how other developers overcome these problems.

I think that the ideal solution would be to either provide a template repository, which can be cloned and used as a sample to create a new syntax repository, or (better) a Node command to create a scaffold repository (maybe with some questions asked, like name of the syntax, etc., to automagically rename folders and fill-in some settings). The latter could either be a tool inside an HJS subfolder, or a proper NPM package that one can install separately.

You read my mind :)

I noticed that the Razor CSHTML is no longer empty, so I cold look at that as an example to start with.

Yeah, looks like that repo is absolutely heading in the right direction!

I appreciate you being one of the first to do this. You can can call on me if you need anything and I'll try to help out as best I can. We also have a Slack channel, if you want to join that. I can send you and invite if you'd like?

@tajmone
Copy link
Contributor Author

tajmone commented Oct 15, 2018

We also have a Slack channel, if you want to join that. I can send you and invite if you'd like?

Please, do!

@tajmone
Copy link
Contributor Author

tajmone commented Oct 15, 2018

@marcoscaceres, I've started populating the Alan repo by using your highlightjs/highlightjs-cshtml-razor project as a reference point.

To keep master branch clean, for now I'm pushing to a temporary branch initial-setup so I can have more freedom to change things before squashing it as a single commit into master.

So, hopefully this can be a starting point. I'll now try to play around with it following your guidelines. I've also found some uncovered edge-cased in the Alan syntax which I should fix, so I'll try to do that using the new system.

I have a few feedback considerations:

  • Highlight.js' .giattributes file should cover in more detail EOL normalization, to prevent cross-platform conflicts, especially if these subprojects' files have to be copied upsteam:
    • every (text) file extension used in HJS should be explicitly defined.
    • Files like LICENSE should be set to native EOL too, IMO.
  • Usually, in my projects, I explicitly set to native EOL every text file, unless it must strictly follow an EOL convention — but knowing which EOL is best practice is not always an easy guess. HJS built packages usually use LF as EOL, so often I have to create a separate pattern to allow programmaticaly including them in other projects.
  • After npm install I found the package-lock.json file in the folder; I wasn't sure if I should exclude it or not (in your project is neither present nor gitignored), so I left it since NPM docs advise to do so.

@marcoscaceres
Copy link
Contributor

Highlight.js' .giattributes file should cover in more detail EOL normalization,

Unix is generally preferred, but in most cases it's best to just let git do the right thing.

Files like LICENSE should be set to native EOL too, IMO.

As above.

After npm install I found the package-lock.json file in the folder; I wasn't sure if I should exclude it or not (in your project is neither present nor gitignored), so I left it since NPM docs advise to do so.

Yes, please use it. We will soon add a package-lock.json. hljs has been supporting old npm version that were not package-lock.json aware, but that should no longer be the case.

@marcoscaceres
Copy link
Contributor

Please ping me over in the new repository when you are ready to do a PR. I'll review your changes then.

@tajmone
Copy link
Contributor Author

tajmone commented Oct 16, 2018

I haven't yet merged the work branch into master because I didn't manage to make the tests work. I've tried running on Node:

import hljs from 'highlight.js/lib/highlight';
import alan from '../highlightjs-alan/alan';
hljs.registerLanguage('alan', alan);

but I keep getting an error at

(function (exports, require, module, __filename, __dirname) { import hljs from 'highlight.js/lib/highlight';
                                                                     ^^^^

SyntaxError: Unexpected identifier
    at new Script (vm.js:79:7)
    at createScript (vm.js:251:10)
    at Object.runInThisContext (vm.js:303:10)
    at Module._compile (internal/modules/cjs/loader.js:657:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:279:19)

HJS was installed alright via NPM, and the path is pointing to the right place. I've looked up the HJS documentation too, and it bascially confirms that it should be working this way — at least on CommonJS, it says.

I've never experience problems building HJS packages via Node.js, so I'm stuck on this error. Definitely, it seems a lot more complicate than the usual fork and pull request system.

@marcoscaceres
Copy link
Contributor

marcoscaceres commented Oct 17, 2018

Oh, import is still experimental in Node and not enabled by default.

Change back to ol' school:

const hljs = require('highlight.js/lib/highlight');
const alan = require('../highlightjs-alan/alan');
hljs.registerLanguage('alan', alan);

@marcoscaceres
Copy link
Contributor

fixed typo above.

@tajmone
Copy link
Contributor Author

tajmone commented Oct 17, 2018

The const fix solved the original problem, but now I'm getting an error when the script gets to the first line of alan.js (function(hljs) {):

SyntaxError: Unexpected token (
    at new Script (vm.js:79:7)
    at createScript (vm.js:251:10)
    at Object.runInThisContext (vm.js:303:10)
    at Module._compile (internal/modules/cjs/loader.js:657:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)

I'm working in test folder where I've installed HLJ (via npm install highlight.js --save) and created a test script:

const hljs = require('highlight.js/lib/highlight');
const alan = require('../highlightjs-alan/alan');
hljs.registerLanguage('alan', alan);

It's the third line that causes the error above (if I comment it out the script exits without error).

Node.js v10.11.0.

@cyblue9
Copy link
Contributor

cyblue9 commented Nov 15, 2018

Hi, @tajmone

I'm faced with same problem.

The const fix solved the original problem, but now I'm getting an error when the script gets to the first line of alan.js (function(hljs) {):

By trial and error, I finished creating highlightjs-xtlang.

Could you look my code?
There are some hint to overcome the problem, maybe. (If there is something you do not understand, please listen to anything.)

Good Luck!!!

@tajmone
Copy link
Contributor Author

tajmone commented Nov 15, 2018

@cyblue9:

By trial and error, I finished creating highlightjs-xtlang.
Could you look my code?
There are some hint to overcome the problem, maybe. (If there is something you do not understand, please listen to anything.)

Thanks a lot, I really appreaciate it! I will look into indeed.

@marcoscaceres
Copy link
Contributor

Closing as moved to https://github.com/highlightjs/highlightjs-alan

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.

None yet

3 participants