todvora / gitbook-plugin-image-captions

Image captions plugin for GitBook
Apache License 2.0
44 stars 18 forks source link

Indexing of levels changed, breaking tests in gitbook@3.0 #12

Closed todvora closed 8 years ago

todvora commented 8 years ago

In the Gitbook 3.0 the indexing of levels has been changed due to parts feature in pages.

This makes indexing of levels different between 2.x and 3.0 releases and breaks the tests.

The real problem is, that we use those level numbers as indexes for additional images configuration - see Image specific captions or Additional image attributes.

There are basically three things we can do about it:

Any ideas, how to solve it? I would really appreciate any feedback on this before changing the behaviour.

CC: @markomanninen

markomanninen commented 8 years ago

I have tried to handle it with an extra configuration parameter something like: removeFirstPartsInSectionNumber and then on the code:

var level = page.level;
if (removeFirstPartsInSectionNumber) {
    level = level.replace(/^1./, '');
}

But this is just a first aid fix at the moment, lets think other options...

todvora commented 8 years ago

Maybe a much cleaner and simpler solution would be to maintain two separate branches of the plugin. One up-to-date with 3.x compatibility only, one for 2.x version of gitbook, where only bugfixes come. Gitbook itself should be already capable to decide, which version should be installed. In package.json of the plugin, following section handles compatibility:

"engines": {
    "gitbook": ">1.x.x"
  }
todvora commented 8 years ago

Ok, so I just created a branch gitbook-2. It holds only support for versions 1.x and 2.x of Gitbook. The code has been cleaned up and all the tests are passing.

Releases could continue with the 0.x numbering or skip directly to 2.x numbers, to follow Gitbook release numbering. Master branch should serve as a base for 3.x compatibility, where all backward-compatible code will be removed. Releases of this branch could either start with 1.x or 3.x. Which seems more logical and clear to you?

markomanninen commented 8 years ago

I tend to think that 2.x.y for GitBook 1 & 2 and 3.x.y for GitBook 3 is easy to remember. Also when major release number is used, then backward compability is not required by a general naming convention.

Reminds me of some Python libraries that had number 3 on names or release numbers when they were updated to support version Python 3.

But this is just a matter of my own taste :)

todvora commented 8 years ago

Hi Marko, You are right, if we increment major versions, we are not required to keep any compatibility with previous versions. Plus it will be easier to remember and also aligned with Gitbook versions.

The 0.x version doesn't make sense anyway, since the plugin is already long time past the initial development phase.

So it could solve several problems at the same time. In the following days, I'll prepare the repo layout and sources.

Thanks for your input, I really appreciate it!

todvora commented 8 years ago

The [current master]() contains now only 3.x compatible code. All the tests has been updated to use new level indexing, which isn't backwards compatible. Only one test is failing. It is caused by incorrect target="_blank" attribute added by Gitbook (pull-request already sent).

I believe that there is nothing preventing us to release two new major releases 2.0 and 3.0.

todvora commented 8 years ago

Both 2.0.0 and 3.0.0 released. I think we are on a good track. Closing this now, thanks for your help @markomanninen!

markomanninen commented 8 years ago

No problem, glad if I could help a bit on the journey. And again, nice work @todvora !