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

Ghost and emacs backup files #2459

Closed
LeartS opened this issue Mar 20, 2014 · 13 comments · Fixed by TryGhost/express-hbs#93 or #6745
Closed

Ghost and emacs backup files #2459

LeartS opened this issue Mar 20, 2014 · 13 comments · Fixed by TryGhost/express-hbs#93 or #6745
Labels
bug [triage] something behaving unexpectedly

Comments

@LeartS
Copy link

LeartS commented Mar 20, 2014

It seems that Ghost templating system reads emacs backup files: they are, by default, file with the same filename of the original file with a tilde (~) as suffix.

Whenever I edit a theme my changes aren't taken in effect until I delete the backup file or rename it, seems like ghost reads from it instead of the regular file.

@ErisDS
Copy link
Member

ErisDS commented Mar 24, 2014

I have created two files: ~post.hbs and ~page.hbs in my theme - I tried creating them whilst Ghost was running, and also restarting Ghost. Those files are in no-way impacting on my theme - it continues to function correctly.

Therefore the odd behaviour you're seeing is not because the Ghost template system is reading ~ files and must be due to some other quirk of emacs. I'm really not sure what we can do here - have you tried just restarting Ghost rather than tampering with the files? Have you tried using a watch task on your theme files to get Ghost to reload?

@LeartS
Copy link
Author

LeartS commented Apr 15, 2014

Suffix, not prefix (post.hbs~).
I did try to reload and restart ghost, refresh ignoring the cache, etc.

I've since moved emacs backup files to a specific folder instead using the same folder, so I don't incur in the problem anymore. I still do not know if it was there in the first place or it was just me doing something stupid.

Maybe it was related to how emacs backups file by default: it renames them and then creates a new one with the original filename, which means that hard links to the file will point to the backup file instead of the new one.

All things considered, if you try file~ (tilde at the end) and don't incur in the problem, I think this issue can be closed as invalid.

@ErisDS ErisDS added this to the Future milestone Apr 16, 2014
@ErisDS
Copy link
Member

ErisDS commented Apr 16, 2014

Sorry yes, I thought the issue said prefix, but yes I can reproduce the problem with a suffix.

I am not sure it is anything to do with hard links, as this occurs after uploading a theme to Ghost(Pro). I think the ~ convention is used by a number of editors. We should look to resolve the bug, but I'm not sure if it is a problem in Ghost core, in express-hbs or in express?

@LeartS
Copy link
Author

LeartS commented Apr 16, 2014

The hard link thing was just an hypotesis for an explanation in case you couldn't reproduce the problem with a suffix, but seeing that you can, this is a valid bug as stated in my first post and we can ignore what I said in my second one.

I do not know where the bug is located, I could try and look into it when I have some spare time.
Thanks for your replies and I am glad I reported a valid bug and not a mistake on my part :)

@sbouafif
Copy link

sbouafif commented May 5, 2014

For anyone using emacs (while the bug is not fixed) you can put all your backup files (the one finishing with ~) in a specific folder or disable this feature. https://stackoverflow.com/questions/2680389/how-to-remove-all-files-ending-with-made-by-emacs

@shindakun
Copy link
Contributor

@ErisDS would you happen to have the zip you tested this with? Was going to look into this but, I can't seem to recreate the issue.

@ErisDS ErisDS modified the milestones: 0.5.x Backlog, Future Backlog Sep 2, 2014
@ErisDS ErisDS added the good first issue [triage] Start here if you've never contributed before. label May 25, 2015
@ErisDS
Copy link
Member

ErisDS commented May 25, 2015

Adding the beginner label to this because I think it ought to be pretty straightforward to fix.

I don't have an example zip, but it's easy enough to reproduce by creating a file with a matching name to a template, but which ends in ~ and putting the wrong content in it.

I believe these ~ files are created by emacs, gedit and brackets as well.

@k-j-kleist
Copy link

Just a guess after some code scrutinization, but no debugging:

The cause might be that ~ is an operator in JavaScript, namely "Bitwise NOT" https://es5.github.io/#x11.4.8

If we follow this calling sequence (sort of) ...

https://github.com/TryGhost/Ghost/blob/stable/core/server/controllers/frontend.js#L136

/*
* Sets the response context around a post and renders it
* with the current theme's post view. Used by post preview
* and single post methods.
* Returns a function that takes the post to be rendered.
*/
function renderPost(req, res) {
    return function (post) {
        return getActiveThemePaths().then(function (paths) {
            var view = template.getThemeViewForPost(paths, post),
                response = formatResponse(post);

            setResponseContext(req, res, response);
            res.render(view, response);
        });
    };
}

https://github.com/TryGhost/Ghost/blob/stable/core/server/controllers/frontend.js#L116

/**
 * Returns the paths object of the active theme via way of a promise.
 * @return {Promise} The promise resolves with the value of the paths.
 */
function getActiveThemePaths() {
    return api.settings.read({
        key: 'activeTheme',
        context: {
            internal: true
        }
    }).then(function (response) {
        var activeTheme = response.settings[0],
            paths = config.paths.availableThemes[activeTheme.value];

        return paths;
    });
}

https://github.com/TryGhost/Ghost/blob/stable/core/server/helpers/template.js#L32

// Given a theme object and a post object this will return
// which theme template page should be used.
// If given a post object that is a regular post
// it will return 'post'.
// If given a static post object it will return 'page'.
// If given a static post object and a custom page template
// exits it will return that page.
templates.getThemeViewForPost = function (themePaths, post) {
    var customPageView = 'page-' + post.slug,
        view = 'post';

    if (post.page) {
        if (themePaths.hasOwnProperty(customPageView + '.hbs')) {
            view = customPageView;
        } else if (themePaths.hasOwnProperty('page.hbs')) {
            view = 'page';
        }
    }

    return view;
};

https://github.com/TryGhost/Ghost/blob/stable/core/server/config/index.js#L71

ConfigManager.prototype.init = function (rawConfig) {
    var self = this;

    // Cache the config.js object's environment
    // object so we can later refer to it.
    // Note: this is not the entirety of config.js,
    // just the object appropriate for this NODE_ENV
    self.set(rawConfig);

    return Promise.all([requireTree(self._config.paths.themePath), requireTree(self._config.paths.appPath)]).then(function (paths) {
        self._config.paths.availableThemes = paths[0];
        self._config.paths.availableApps = paths[1];
        return self._config;
    });
};

https://github.com/TryGhost/Ghost/blob/stable/core/server/require-tree.js#L52 (buckle up! I haven't even attempted to grok this code... )

readDir = function (dir, options, depth, messages) {
        depth = depth || 0;
        messages = messages || {
            errors: [],
            warns: []
        };

        options = _.extend({
            index: true,
            followSymlinks: true
        }, options);

        if (depth > 1) {
            return Promise.resolve(null);
        }

        return readdirAsync(dir).then(function (files) {
            files = files || [];

            return Promise.reduce(files, function (results, file) {
                var fpath = path.join(dir, file);

                return lstatAsync(fpath).then(function (result) {
                    if (result.isDirectory()) {
                        return readDir(fpath, options, depth + 1, messages);
                    } else if (options.followSymlinks && result.isSymbolicLink()) {
                        return readlinkAsync(fpath).then(function (linkPath) {
                            linkPath = path.resolve(dir, linkPath);

                            return lstatAsync(linkPath).then(function (result) {
                                if (result.isFile()) {
                                    return linkPath;
                                }

                                return readDir(linkPath, options, depth + 1, messages);
                            });
                        });
                    } else if (depth === 1 && file === 'package.json') {
                        return parsePackageJson(fpath, messages);
                    } else {
                        return fpath;
                    }
                }).then(function (result) {
                    results[file] = result;

                    return results;
                });
            }, {});
        });
    },
    readAll = function (dir, options, depth) {
        // Start with clean messages, pass down along traversal
        var messages = {
            errors: [],
            warns: []
        };

        return readDir(dir, options, depth, messages).then(function (paths) {
            // for all contents of the dir, I'm interested in the ones that are directories and within /theme/
            if (typeof paths === 'object' && dir.indexOf('theme') !== -1) {
                _.each(paths, function (path, index) {
                    if (typeof path === 'object' && !path.hasOwnProperty('package.json') && index.indexOf('.') !== 0) {
                        messages.warns.push({
                            message: 'Found a theme with no package.json file',
                            context: 'Theme name: ' + index,
                            help: 'This will be required in future. Please see http://docs.ghost.org/themes/'
                        });
                    }
                });
            }

... we might come to the conclusion that the issue has to do with the fact that filenames are used as properties on _config.paths.availableThemes, and if a filename ends with ~ strange things might happen.

Finally some food for thought: https://www.safaribooksonline.com/library/view/you-dont-know/9781491905159/ch04.html ("The curious case of the ~")

@jgillich
Copy link
Contributor

I was not able to reproduce this by creating a post.hbs~. So either it got fixed or I'm doing something wrong?

@k-j-kleist
Copy link

I had forgotten about this issue, and was recently almost driven mad by the behavior of Ghost.

I was editing .../my-theme/partials/loop.hbs but the rendered page was not modified, although I run Ghost in development mode. And yes, I cleared my browser cache. And I restarted Ghost, to no avail. As if the handlebars engine would cache the partial in some odd filesystem directory.

Guess which editor I was using? (evil people claim that its name is an acronym for escape-meta-alt-control-shift)

So I removed the backup file loop.hbs~, restarted Ghost in development mode, and all was fine.

Not.

Uh oh: there was also a loop.hbs-ORIG in partials/, manually created by myself (no git here and now). I renamed it to loop-hbs-ORIG and everything worked. Really.

So: This issue should be renamed to something like Non .hbs files are consumed as if they were handlebars templates. This has nothing to do with emacs and ~. (And my theory above is moot.)

Ghost: 0.7.1, nodejs: 0.12.2, CentOS release 6.7

@olsio
Copy link
Contributor

olsio commented Oct 6, 2015

I found the root cause in the express-hbs dependency. The caching layer is loading partials as . which can cause the application of a partial twice overriding the expected version.

Fix submitted:
https://github.com/barc/express-hbs/pull/88/files

@olsio
Copy link
Contributor

olsio commented Oct 21, 2015

I tried to get a fix into the express-hbs module but this would break the current accepted behavior of partials being any kind of file.

To be at least aware I added a new validation check for partials that resolve to the same name. Ghost will give a warning when it finds such a conflict.

@ErisDS ErisDS removed the good first issue [triage] Start here if you've never contributed before. label Jan 11, 2016
@jayjayjpg
Copy link

This issue happens to me with as well in this setup:

Ubuntu LTS 14.04
Kate 3.13.3 (based on KDE Dev Platform 4.13.3)
Ghost 0.7.0
Nodejs 0.10.0 || ~ 0.12.0

In the default config of the editor Kate, for every file that has been edited and deleted in the file browser, a temporary backup file with the extension ~ will be created and has to be deleted manually for changes in ghost to take effect.

ErisDS pushed a commit to ErisDS/express-hbs that referenced this issue Apr 20, 2016
closes TryGhost/Ghost#2459

cachePartials was loading partials as *.* which was overriding the
expected layouts with emacs backup file or other stale versions.

Fix is taking the file extension from the options object.
ErisDS added a commit to ErisDS/Ghost that referenced this issue Apr 21, 2016
refs TryGhost#2459

This includes updating handlebars to v4 as well as other fixes which
will introduce breaking changes to the theme API:

From express-hbs:
- partials must now use the `.hbs` extension, the same as templates
From handlebars:
- Depthed paths are now conditionally pushed on to the stack.
If the helper uses the same context, then a new stack is not created.
This leads to behavior that better matches expectations for helpers
like if that do not seem to alter the context. Any instances of ../
in templates will need to be checked for the correct behavior under 4.0.0.
In general templates will either reduce the number of ../ instances
or leave them as is. See TryGhost#1028.
- The = character is now HTML escaped. This closes a potential exploit
case when using unquoted attributes, i.e. <div foo={{bar}}>. In general
it's recommended that attributes always be quoted when their values are
generated from a mustache to avoid any potential exploit surfaces.
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this issue Nov 20, 2016
refs TryGhost#2459

This includes updating handlebars to v4 as well as other fixes which
will introduce breaking changes to the theme API:

From express-hbs:
- partials must now use the `.hbs` extension, the same as templates
From handlebars:
- Depthed paths are now conditionally pushed on to the stack.
If the helper uses the same context, then a new stack is not created.
This leads to behavior that better matches expectations for helpers
like if that do not seem to alter the context. Any instances of ../
in templates will need to be checked for the correct behavior under 4.0.0.
In general templates will either reduce the number of ../ instances
or leave them as is. See TryGhost#1028.
- The = character is now HTML escaped. This closes a potential exploit
case when using unquoted attributes, i.e. <div foo={{bar}}>. In general
it's recommended that attributes always be quoted when their values are
generated from a mustache to avoid any potential exploit surfaces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly
Projects
None yet
8 participants