search

openzim / node-libzim

Libzim binding for Node.js: read/write ZIM files in Javascript
https://www.npmjs.com/package/@openzim/libzim
GNU General Public License v3.0
27 stars 11 forks source link

Articles and `shouldIndex` method. #30

Closed mgautierfr closed 4 years ago

mgautierfr commented 5 years ago

I'm not sure it is a bug. But I prefer to speak about it for nothing than be quite when I should speak :)

With the last change in libzim 6.0.0, the shouldIndex should also return true for redirect article if needed. "If needed" means that if the user can search for the article (probably any html article, but not mandatory). Whatever if the article is a plain article or a redirect one.

kelson42 commented 5 years ago

@ISNIT0 What @mgautierfr says sounds correct. Could we fix it? What is the current impact of not doing it properly?

kelson42 commented 4 years ago

@mgautierfr Is that still an actual bug? Notnsure I fully understand it anymore.

mgautierfr commented 4 years ago

I don't know. Does node-libzim correctly return true for shouldIndex if the article has to be indexed ?

kelson42 commented 4 years ago

@mgautierfr I don't know... but I don't have complained about the problem. It is anyway not the role of node-libzim to decide this I believe.

kelson42 commented 4 years ago

@mgautierfr Does that mean MWoffliner works properly here? https://github.com/openzim/mwoffliner/blob/master/src/mwoffliner.lib.ts#L477

mgautierfr commented 4 years ago

It seems you always return true for redirect articles. If redirect articles are always article to be indexed you are good. Else, you should return true only if you want to index them.

kelson42 commented 4 years ago

@mgautierfr Looks good to me.