raghur / vim-ghost

Vim/Nvim client for GhostText - Edit browser text areas in Vim/Neovim
333 stars 13 forks source link

Remove unsupported parameter max_length of slugify() #7

Closed mcepl closed 6 years ago

mcepl commented 6 years ago

Not only slugify() doesn't accept max_length parameter, but it is completely unnecessary. Good old Python slice works perfectly for limiting string to maximal length, because if mystring is shorter than MAX_LENGTH then mystring[:MAX_LENGTH] just does nothing.

Fixes #3

raghur commented 6 years ago

Thank you for the PR.

mcepl commented 6 years ago

And BTW, given the complexity of the module slugify (33 lines wc -l even in the straddling mode with all blank lines and comments) and quality of its support (even pull request from 2012-06-22 has not been merged yet), I would very hesitantly suggest bundling the function in.

Or, you can go even all the way:

                prefix = "ghost-" + req["url"] + "-" + \
                    req["title"].encode('ascii', 'ignore')[:50]

(given req originally comes from json.loads all its text values are unicode (or str in Python 3) so it is unnecessary to dance around possibility of getting bytes there)

raghur commented 6 years ago

Here's the python slugify repo -https://github.com/un33k/python-slugify . Your link points to a different repo (not the one used here)

mcepl commented 6 years ago

sorry, you are too fast. I have updated my comment above.

And yes, you are right, it is another module. Even worse, why to get to all this length because of something which could be completely replaced by oneliner?

raghur commented 6 years ago

And yes, you are right, it is another module. Even worse, why to get to all this length because of something which could be completely replaced by oneliner?

Simply because page titles can have all sorts of unicode characters and at the time, python slugify seemed a good way to handle that.

mcepl commented 6 years ago

Simply because page titles can have all sorts of unicode characters and at the time, python slugify seemed a good way to handle that.

Well, the worst thing which can happen with my solution is that the slug comes out empty, which is not that bad thing considering all previous text before it. Or you can check for it and in case the slug is empty, use some totally random string.