Skip to content

Commit

Permalink
Reorganise variable processing (part 1)
Browse files Browse the repository at this point in the history 
Processing variables involves a lot of boilerplate code with minor
variations. This decentralizes variable processing, decreasing code
maintainability and readability.

Let's embark on a two stage refactor to improve variable processing.
In this first refactor, we create the variablePreprocessor abstraction,
and completely migrate the use of userDefinedVariablesMap for site
variables to variablePreprocessor, reducing boilerplate code relating
to site variables.
Additionally, we move all page variable processing related methods to
variablePreprocessor from Parser, abstracting such methods into
smaller units of functionality where possible, and enhancing in code
documentation.

In the second part, we will similarly refactor page variable processing
methods to reduce more boilerplate, increasing code maintainability.
  • Loading branch information
@ang-zeyu
ang-zeyu committed May 5, 2020
1 parent a515669 commit 211bc3f
Show file tree
Hide file tree
Showing 7 changed files with 557 additions and 337 deletions.
24 changes: 10 additions & 14 deletions src/Page.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Page {
* @property {string} pageTemplate template used for this page
* @property {string} title
* @property {string} titlePrefix https://markbind.org/userGuide/siteConfiguration.html#titleprefix
* @property {Object<string, any>} userDefinedVariablesMap
* @property {VariablePreprocessor} variablePreprocessor
* @property {string} sourcePath the source file for rendering this page
* @property {string} tempPath the temp path for writing intermediate result
* @property {string} resultPath the output path of this page
Expand Down Expand Up @@ -167,9 +167,9 @@ class Page {
*/
this.titlePrefix = pageConfig.titlePrefix;
/**
* @type {Object<string, any>}
* @type {VariablePreprocessor}
*/
this.userDefinedVariablesMap = pageConfig.userDefinedVariablesMap;
this.variablePreprocessor = pageConfig.variablePreprocessor;

/**
* The source file for rendering this page
Expand Down Expand Up @@ -643,8 +643,7 @@ class Page {
this.includedFiles.add(headerPath);
// Map variables
const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap);
const userDefinedVariables = this.userDefinedVariablesMap[parentSite];
return `${njUtil.renderRaw(headerContent, userDefinedVariables)}\n${pageData}`;
return `${this.variablePreprocessor.renderSiteVariables(parentSite, headerContent)}\n${pageData}`;
}

/**
Expand Down Expand Up @@ -673,8 +672,7 @@ class Page {
this.includedFiles.add(footerPath);
// Map variables
const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap);
const userDefinedVariables = this.userDefinedVariablesMap[parentSite];
return `${pageData}\n${njUtil.renderRaw(footerContent, userDefinedVariables)}`;
return `${pageData}\n${this.variablePreprocessor.renderSiteVariables(parentSite, footerContent)}`;
}

/**
Expand Down Expand Up @@ -710,8 +708,7 @@ class Page {
this.includedFiles.add(siteNavPath);
// Map variables
const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap);
const userDefinedVariables = this.userDefinedVariablesMap[parentSite];
const siteNavMappedData = njUtil.renderRaw(siteNavContent, userDefinedVariables);
const siteNavMappedData = this.variablePreprocessor.renderSiteVariables(parentSite, siteNavContent);
// Convert to HTML
const siteNavDataSelector = cheerio.load(siteNavMappedData);
if (siteNavDataSelector('navigation').length > 1) {
Expand Down Expand Up @@ -847,8 +844,7 @@ class Page {
this.includedFiles.add(headFilePath);
// Map variables
const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap);
const userDefinedVariables = this.userDefinedVariablesMap[parentSite];
const headFileMappedData = njUtil.renderRaw(headFileContent, userDefinedVariables)
const headFileMappedData = this.variablePreprocessor.renderSiteVariables(parentSite, headFileContent)
.trim();
// Split top and bottom contents
const $ = cheerio.load(headFileMappedData, { xmlMode: false });
Expand Down Expand Up @@ -907,7 +903,7 @@ class Page {
* @typedef {Object<string, any>} FileConfig
* @property {Set<string>} baseUrlMap the set of urls representing the sites' base directories
* @property {string} rootPath
* @property {Object<string, any>} userDefinedVariablesMap
* @property {VariablePreprocessor} variablePreprocessor
* @property {Object<string, number>} headerIdMap
* @property {boolean} fixedHeader indicates whether the header of the page is fixed
*/
Expand All @@ -925,7 +921,7 @@ class Page {
const fileConfig = {
baseUrlMap: this.baseUrlMap,
rootPath: this.rootPath,
userDefinedVariablesMap: this.userDefinedVariablesMap,
variablePreprocessor: this.variablePreprocessor,
headerIdMap: this.headerIdMap,
fixedHeader: this.fixedHeader,
};
Expand Down Expand Up @@ -1241,7 +1237,7 @@ class Page {
});
return markbinder.includeFile(dependency.to, {
baseUrlMap: this.baseUrlMap,
userDefinedVariablesMap: this.userDefinedVariablesMap,
variablePreprocessor: this.variablePreprocessor,
rootPath: this.rootPath,
cwf: file,
}).then(result => Page.removeFrontMatter(result))
Expand Down
49 changes: 21 additions & 28 deletions src/Site.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const path = require('path');
const Promise = require('bluebird');
const ProgressBar = require('progress');
const walkSync = require('walk-sync');
const MarkBind = require('./lib/markbind/src/parser');
const VariablePreprocessor = require('./lib/markbind/src/preprocessors/variablePreprocessor');
const njUtil = require('./lib/markbind/src/utils/nunjuckUtils');
const injectHtmlParser2SpecialTags = require('./lib/markbind/src/patches/htmlparser2');
const injectMarkdownItSpecialTags = require(
Expand Down Expand Up @@ -134,7 +134,9 @@ class Site {
*/
this.siteConfig = undefined;
this.siteConfigPath = siteConfigPath;
this.userDefinedVariablesMap = {};

// Site wide variable preprocessor
this.variablePreprocessor = undefined;

// Lazy reload properties
this.onePagePath = onePagePath;
Expand Down Expand Up @@ -271,7 +273,7 @@ class Site {
title: config.title || '',
titlePrefix: this.siteConfig.titlePrefix,
headingIndexingLevel: this.siteConfig.headingIndexingLevel,
userDefinedVariablesMap: this.userDefinedVariablesMap,
userDefinedVariablesMap: this.variablePreprocessor,
sourcePath,
resultPath,
asset: {
Expand Down Expand Up @@ -513,6 +515,7 @@ class Site {
.map(x => path.resolve(this.rootPath, x));

this.baseUrlMap = new Set(candidates.map(candidate => path.dirname(candidate)));
this.variablePreprocessor = new VariablePreprocessor(this.rootPath, this.baseUrlMap);

return Promise.resolve();
}
Expand All @@ -523,12 +526,9 @@ class Site {
collectUserDefinedVariablesMap() {
// The key is the base directory of the site/subsites,
// while the value is a mapping of user defined variables
this.userDefinedVariablesMap = {};
const markbindVariable = { MarkBind: MARKBIND_LINK_HTML };
this.variablePreprocessor.resetUserDefinedVariablesMap();

this.baseUrlMap.forEach((base) => {
const userDefinedVariables = {};
Object.assign(userDefinedVariables, markbindVariable);
const userDefinedVariablesPath = path.resolve(base, USER_VARIABLES_PATH);
const userDefinedVariablesDir = path.dirname(userDefinedVariablesPath);
let content;
Expand All @@ -539,34 +539,31 @@ class Site {
logger.warn(e.message);
}

// This is to prevent the first nunjuck call from converting {{baseUrl}} to an empty string
// and let the baseUrl value be injected later.
userDefinedVariables.baseUrl = '{{baseUrl}}';
this.userDefinedVariablesMap[base] = userDefinedVariables;
/*
This is to prevent the first nunjuck call from converting {{baseUrl}} to an empty string
and let the baseUrl value be injected later.
*/
this.variablePreprocessor.addUserDefinedVariable(base, 'baseUrl', '{{baseUrl}}');
this.variablePreprocessor.addUserDefinedVariable(base, 'MarkBind', MARKBIND_LINK_HTML);

const $ = cheerio.load(content);
$('variable,span').each(function () {
const name = $(this).attr('name') || $(this).attr('id');
const variableSource = $(this).attr('from');
$('variable,span').each((index, element) => {
const name = $(element).attr('name') || $(element).attr('id');
const variableSource = $(element).attr('from');

if (variableSource !== undefined) {
try {
const variableFilePath = path.resolve(userDefinedVariablesDir, variableSource);
const jsonData = fs.readFileSync(variableFilePath);
const varData = JSON.parse(jsonData);
Object.entries(varData).forEach(([varName, varValue]) => {
// Process the content of the variable with nunjucks, in case it refers to other variables.
const variableValue = njUtil.renderRaw(varValue, userDefinedVariables, {}, false);

userDefinedVariables[varName] = variableValue;
this.variablePreprocessor.addAndRenderUserDefinedVariable(base, varName, varValue);
});
} catch (err) {
logger.warn(`Error ${err.message}`);
}
} else {
// Process the content of the variable with nunjucks, in case it refers to other variables.
const html = njUtil.renderRaw($(this).html(), userDefinedVariables, {}, false);
userDefinedVariables[name] = html;
this.variablePreprocessor.addAndRenderUserDefinedVariable(base, name, $(element).html());
}
});
});
Expand Down Expand Up @@ -691,7 +688,7 @@ class Site {
const filePathArray = Array.isArray(filePaths) ? filePaths : [filePaths];
const uniquePaths = _.uniq(filePathArray);
logger.info('Rebuilding affected source files');
MarkBind.resetVariables();
this.variablePreprocessor.resetVariables();
return new Promise((resolve, reject) => {
this.regenerateAffectedPages(uniquePaths)
.then(() => fs.removeAsync(this.tempPath))
Expand All @@ -709,7 +706,7 @@ class Site {
const uniqueUrls = _.uniq(normalizedUrlArray);
uniqueUrls.forEach(normalizedUrl => logger.info(
`Building ${normalizedUrl} as some of its dependencies were changed since the last visit`));
MarkBind.resetVariables();
this.variablePreprocessor.resetVariables();

/*
Lazy loading only builds the page being viewed, but the user may be quick enough
Expand All @@ -729,7 +726,6 @@ class Site {
}

this.toRebuild.delete(normalizedUrl);
pageToRebuild.userDefinedVariablesMap = this.userDefinedVariablesMap;
pageToRebuild.generate(new Set())
.then(() => {
pageToRebuild.collectHeadingsAndKeywords();
Expand Down Expand Up @@ -1062,7 +1058,6 @@ class Site {
}
}

page.userDefinedVariablesMap = this.userDefinedVariablesMap;
processingFiles.push(page.generate(builtFiles)
.catch((err) => {
logger.error(err);
Expand Down Expand Up @@ -1281,9 +1276,7 @@ class Site {
timeZoneName: 'short',
};
const time = new Date().toLocaleTimeString(this.siteConfig.locale, options);
Object.keys(this.userDefinedVariablesMap).forEach((base) => {
this.userDefinedVariablesMap[base].timestamp = time;
});
this.variablePreprocessor.addUserDefinedVariableForAllSites('timestamp', time);
return Promise.resolve();
}
}
Expand Down
Loading

0 comments on commit 211bc3f

Please sign in to comment.