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

gulp: Comment annotations #1535

Closed

Conversation

RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Aug 22, 2018

This PR replaces the current languages_placeholder with a new comment annotation base system.
This will allow us to more easily add generated content in a more natural and configurable way.

What annotations are

Annotations are single line comments which have the form:

// @annotation
or
// @annotation(options)

Right now, annotations have to be followed by a variable definition, assignment or key-value-pair, like this:

// @lang.test
var testLanguage = content;
// @counter
'counter': 1,

How annotations are replaced

Right now, replaceAnnotations is used to in combination with a replacer function. The replacer will get the name of the annotation, the specified options (optional), and the previous value as arguments. The string returned will replace the value of the variable.

replaceAnnotations is intended to be piped:

return gulp.src(paths)
    .pipe(replaceAnnotations(replacer))
    .pipe(gulp.dest('./build'));

replacer could look like this:

const replacer = (annotation, options, value) => {
    switch (annotation) {
        case 'lang.test':
            return JSON.stringify(languages.test.get(JSON.parse(options || '{}')));
        case 'counter':
            return Number(value) + 1;
        default:
            throw new Error('Unknown annotation: ' + annotation);
    }
};

Some more details

  • Annotation options:
    • If an annotation does not specify options, the replacer will be given undefined.
    • Everything within the round braces will be passed to the replacer. This includes leading and trailing spaces.
  • The previous value:
    • This value does not include leading and trailing spaces.

IMPORTANT: The string returned by a replacer has to be a single line.

New functions

To make asynchronous operations easier to deal with, I added 3 new functions, all of which return a Promise.

  • promiseStream will take a ReadableStream and return a Promise which listens to the streams end event.
  • readString will read a file and return a Promise with the string content of the file.
  • readJson will parse the text from readString and return a Promise with the JSON object,

Other changes

  • gulp now looks through all plugins when searching for annotations.
  • The code in components.js in no longer hard-coded in gulpfile.js.

@mAAdhaTTah
Copy link
Member

I see the improvement, but I'm not really sure we need something more complex than what we have, as we're not doing a lot of generation. We can leave this open as a "maybe later" but this seems unnecessary at this time.

@RunDevelopment
Copy link
Member Author

I'm actually planning for a few other features which will make this completely unnecessary, so I'm closing this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants