Skip to content

Conversation

@Dike90
Copy link
Contributor

@Dike90 Dike90 commented Nov 3, 2018

No description provided.

yajo and others added 8 commits November 3, 2018 14:28
* [8.0][html_text] Excerpt generator.

This module adds a technical utility to allow the developer to get an excerpt from any HTML chunk.

You can choose the maximum amount of words or characters if you want.

See the README and inline docstrings for further details.
OCA Transbot updated translations from Transifex
OCA Transbot updated translations from Transifex
[FIX] Tests

[FIX] Do not test the specific exception

[FIX] Do not test the specific exception
@oca-clabot
Copy link

Hey @Dike90, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

  • @Dike90 (login unknown in OCA database)

Appreciation of efforts,
OCA CLAbot

@bigteethk
Copy link

LGTM

@pedrobaeza pedrobaeza added this to the 11.0 milestone Nov 3, 2018
@OCA-git-bot OCA-git-bot mentioned this pull request Nov 3, 2018
32 tasks
Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pedrobaeza
Copy link
Member

@Dike90 have you signed the CLA?

Copy link
Member

@fkantelberg fkantelberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review

return ""

# Get words
words = u"".join(doc.xpath("//text()")).split()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause a problem if the html isn't using spaces around tags. e.g. <a>Test</a><b>Hello</b>. Best would be to use a space for joining.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yajo any thoughts about this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree, because i.e. This is a <b>test</b> still has an ending space after "a". On the other hand My<b>Long</b>Word is still one word.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then is it correct for all cases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's ok as it is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You get problems when you use something like <div>, <p> or <td> tags. e.g. <div>Hello,</div><div>Ok</div>. But of course it's probably a edge case and can be fixed with html formatting.

Copy link
Member

@cristinamartinrod cristinamartinrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yajo yajo merged commit ba8ddde into OCA:12.0 Feb 18, 2019
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (12.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.