assemble / grunt-assemble-i18n

Assemble middleware for adding i18n support to projects.
24 stars 8 forks source link

Suggestion: make replacement of page file name optional #41

Open mauron85 opened 10 years ago

mauron85 commented 10 years ago

Currently plugin generates files in format: page-language.ext So resulting site looks like this:

[en]
index-en.html
blog-en.html

IMHO adding lang suffix to filename is not necessary and should be optional.

For me fix is replacing line 21 In lib/i18n.js: from:

var filename = page.replace(ext, "-" + language + ext);

to:

var filename = page;
mauron85 commented 10 years ago

Hmm, I've just realized it's not that easy fix. Now it's not generating all languages from i18n.json, only the last one. And index.html is in root not in lang subfolder.

Assembling dist/sk/blog.html OK
Assembling dist/sk/contact.html OK
Assembling dist/index.html OK
Assembling dist/sk/pricing.html OK
doowb commented 10 years ago

Take a look at this example in the Gruntfile.js. It uses permalinks to generate a nicer url. If you're use this make sure to install assemble-contrib-permalinks.

ain commented 10 years ago

Yup. It was discussed and decided that this part is better off with assemble-contrib-permalinks offering better separation of responsibilities :)

mauron85 commented 10 years ago

@doowb @ain I'm using assemble-contrib-permalinks already and I've used code from that example.

Fragment from my gruntfile

      "with-permalinks": {
        options: {
          plugins: ['assemble-contrib-i18n', 'assemble-contrib-permalinks'],
          i18n: {
            data: '<%= config.src %>/data/i18n.json',
            templates: ['<%= config.src %>/templates/pages/*.hbs']
          },
          permalinks: {
            structure: ':language/:basename.html'
          }
        },
        dest: '<%= config.dist %>/',
        src: '!*.*'
      },

And actually in example structure has wrong format. It will always generate pages with filename index.html. Anyway I would like to have structure like this (which is currently not possible):

[lang]
index.html
blog.html
...
mauron85 commented 10 years ago

It looks like it has something to do with following issue: https://github.com/assemble/assemble-middleware-permalinks/issues/20

And I can get expected behavior using permalinks replacement function like in pull request https://github.com/assemble/assemble-middleware-permalinks/pull/53

Gruntfile.js

...
"with-permalinks": {
  options: {
    plugins: ['assemble-contrib-i18n', 'assemble-contrib-permalinks'],
    i18n: {
      data: '<%= config.src %>/data/i18n.json',
      templates: ['<%= config.src %>/templates/pages/*.hbs']
    },
    permalinks: {
      structure: ':language/:mybasename.html',
      patterns: [{
        pattern: ':mybasename',
        replacement: function() {
          var basename = this.basename.split('-')[0];
          return basename;
        }
      }]
    }
  },
  dest: '<%= config.dist %>/',
  src: '!*.*'
}
...
jonschlinkert commented 10 years ago

@mauron85 to clarify were you able to get this resolved with the replacement function?

mauron85 commented 10 years ago

YES it completely resolved my issue. So now instead of following struct: en/index-en.html, en/blog-en.html ...

I have: en/index.html, en/blog.html ...

But I still think, that this is just a workaround. I would like to have an config option for that instead.

jonschlinkert commented 10 years ago

A config option for what specifically? how do you think it should be implemented?

mauron85 commented 10 years ago

Generating files into directory structure based on language is enough. Adding extra "lang" suffix to files is not what I want. I've checked your code and I think the process function is responsible for it.

  var process = function (languages) {
    // Iterate over the languages
    languages.forEach(function (language) {

      templates.forEach(function (page) {
        var ext = path.extname(page);
        var pageObj = matter.read(page);

        var context = _.extend({}, {language: language}, pageObj.context);
        var filename = page.replace(ext, "-" + language + ext);

        pages[filename] = {
          filename: filename,
          content: pageObj.content,
          data: context
        };

      });
    });
  };

line:

var filename = page.replace(ext, "-" + language + ext); 

is responsible for adding the suffix. Variable filename is used for generating pages file names. I would like to have filename property to contain unchanged filename based on actual template name. So for template file name index.hbs and lang: EN pages array will look like:

pages['index-en.hbs'] = {filename: 'index.hbs' ...}

Filename property should be used by assemle-contrib-permalinks plugin to generate page file names. Unfortunately current situation is that filename property is completely ignored. Page name is actually generated from pages array (hashmap) instead, where key is the name of page file name.

LaurentGoderre commented 10 years ago

The problem is that there so many different ways to handle multilingual sites out there that creating a configuration would be tricky. That's why I think the permalinks solution is very elegant.

ain commented 10 years ago

Absolutely.

Sent from Gmail Mobile

mauron85 commented 10 years ago

@LaurentGoderre yes permalinks are way to go. I agree. But it's not possible to use them with i18n plugin, because of issue I mentioned before. Look at my sample project. I use permalinks replacement function to get rid of suffixes, but the problem is that in my nav generating partial permalinks still generates links with suffixes.

<li><a href="{{#is this ../language}}#{{else}}/{{this}}/{{../../basename}}{{/is}}">{{this}}</a></li>

I've tried {{../../basename}} and also {{../../filename}}, all without success. It's not possible to switch from [en] to [sk], because links are incorrect.

I think the problem lies in i18n plugin and has to be fixed. I've described it in my previous comment.

LaurentGoderre commented 10 years ago

Very interesting! I will see what I can do

jonschlinkert commented 10 years ago

@mauron85 try using a custom helper. Here is an example (this one is used for logging messages: https://github.com/assemble/assemble.io/blob/v0.3.0/structure/pages/helpers.hbs#L18)

Here are a few different helpers that do something related to renaming: https://github.com/assemble/assemble.io/blob/v0.3.0/structure/_helpers/replace.js (keep in mind you'll need to use the Assemble 0.4.x signature for registering helpers if that's the version you're using).

To use the helper, you would do something like this:

{{rename ../../basename}}

If you have any preferences regarding the type of helper you want to use there, I'd be happy to write one for you. We could do one that uses regex defined on the hash to modify the path, or just one that simply strips extensions.

(edited, a simple helper might be enough)

jonschlinkert commented 10 years ago

@mauron85 is the example project up to date?

mauron85 commented 10 years ago

Yep, all libs should be latest. Projekt is less then week old.

eikawata commented 9 years ago

+1 for this feature. I got hit by the same problem.

olegmeglin commented 8 years ago

+1 Same problem here