naokazuterada / MarkdownTOC

SublimeText3 plugin which generate a table of contents (TOC) in a markdown document.
https://packagecontrol.io/packages/MarkdownTOC
MIT License
301 stars 48 forks source link

You might drop bs4 dependency #111

Closed ziembla closed 6 years ago

ziembla commented 6 years ago

Thanks for a great tool! The beautiful soup wouldn't work for me but I think you could easily drop that additional dependency by using one more regex - the only victim would be "id_replacements" (it'd need double-quotes inside), but they are not only impractical to set in the document even now but also should rather be set user-wide as json in the first place, shouldn't they? All unit tests passing (thanks again - for having them built-in and for the guideline to use them). Regards

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.02%) to 93.293% when pulling cd2f7488ba4ae14f9a4430354a199068b9fa53a2 on ziembla:ziembla-mod into 65693c7390d338eb0d6d8ae19ea5c5a2f5b4fee6 on naokazuterada:master.

ziembla commented 6 years ago

If I am reading logs correctly:

jonasbn commented 6 years ago

@ziembla the increase in lines would explain the decrease in coverage, even if the number of lines covered it the same

naokazuterada commented 6 years ago

@ziembla Hi, thank you for your PR and sorry for my late reply. 😃

I surely agree with your idea that removing dependencies as far as possible, but in this time I want to consider about it because I think bs will provide more useful functions in coming new features. So please tell me about your problem. I'm not sure why bs doesn't work your environment. Is there any special stuff? And what kind of symptom is happening in your place?

ziembla commented 6 years ago

@naokazuterada thanks, now it's me to reply after an even longer delay... I needed to use the addon on the offline computer where the sublime couldn't reach internet to get dependencies (I just copy Packages folder there). I haven't found the easy way to install the package into sublime environment... And as I looked into your source and seen there was only one and simple to change place where bs was used I just did that. If you plan to add some features that would need more of bs power to avoid implementing advanced processing by hand, perhaps you shouldn't care about my scenario. On the other hand I am always a bit sceptical about addons installing their own dependencies as that may easily lead to conflicting version dependencies... (I didn't dig into sublime addon architecture but I don't expect they're isolated python envs?) Regards

naokazuterada commented 6 years ago

@ziembla Hi, thank you for reply! Your case is unexpected for me, but make sense. Sorry I have no idea for dealing with it.

I've considered and decided to drop bs this time. I don't know whether I will revert it or not in the future, but, as you said, it's not necessary so far.

https://github.com/naokazuterada/MarkdownTOC/releases/tag/2.7.1

Thank you for your contribution again! 🏅

ziembla commented 6 years ago

Thanks a lot, I am switching back to your official repo then!

naokazuterada commented 6 years ago

BTW, I've fixed errors on CI. It seems to be caused by travis updates.