Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing bug #537, image filenames and web links are not included in wo… #696

Merged
merged 45 commits into from
May 5, 2024

Conversation

laraconda
Copy link
Contributor

@laraconda laraconda commented May 14, 2023

…rds cloud anymore.

Bug: #537

Summary of the changes in this pull request

  • Added regular expressions to exclude web links and files in the cloud of words.
  • Refactored the function "select_most_frequent_words".
  • TODO

Pull request checklist

  • I have added an entry in CHANGELOG.md including my name and issue and/or pull request number.
  • If applicable: I have removed the corresponding entry in TODO.md.

Copy link
Owner

@jendrikseipp jendrikseipp left a comment

Choose a reason for hiding this comment

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

Looks good to me! I only have cosmetic comments.

rednotebook/gui/clouds.py Outdated Show resolved Hide resolved
rednotebook/gui/clouds.py Outdated Show resolved Hide resolved
rednotebook/gui/clouds.py Outdated Show resolved Hide resolved
@laraconda
Copy link
Contributor Author

I'm thinking about adding small tests for this. I should make a new file in tests/ right? Will tox automatically run it if its there?

@jendrikseipp
Copy link
Owner

Good idea! If it doesn't fit with some other tests that are already there, yes, please add a new test file. Tox will pick it up automatically.

@laraconda
Copy link
Contributor Author

I tried to write a test for this functionality but since the class Cloud takes an instance of journal, I figured it would be complicated to instanciate Journal and insert simple text into it. @jendrikseipp do you think there's an easy way to do it?

rednotebook/gui/clouds.py Outdated Show resolved Hide resolved
rednotebook/gui/clouds.py Outdated Show resolved Hide resolved
rednotebook/gui/clouds.py Outdated Show resolved Hide resolved
@jendrikseipp
Copy link
Owner

I agree that testing this is too complicated. So no need to add a test here.

@jendrikseipp
Copy link
Owner

I'm assuming the ball is in your court. Give me a shout when you want me to review your changes.

laraconda and others added 21 commits May 5, 2024 18:52
…#703)

* Fixing url not recognized when hashtag symbol is followed by slash. Issue jendrikseipp#556
* Adding more cpp directives to hashtag pattern in t2t. Adding comment regarding what each hashtag regex is used for in both files.
---------
Co-authored-by: Jendrik Seipp <jendrikseipp@gmail.com>
…ipp#687)

This can be used to merge one rednotebook directory into another.

---------

Signed-off-by: Simon Glass <sjg@chromium.org>
Co-authored-by: Jendrik Seipp <jendrikseipp@gmail.com>
It is useful to be able to merge notebooks regularly across multiple
machines. The existing script adds the new material again, even if a
previous merge already added it.

Add a check for this, so that repeated merging will not create lots of
duplicate text.

Also include a summary of what happened and mention a script dependency.

Signed-off-by: Simon Glass <sjg@chromium.org>
@jendrikseipp jendrikseipp merged commit e2dcd74 into jendrikseipp:master May 5, 2024
6 of 7 checks passed
@jendrikseipp
Copy link
Owner

Thanks @laraconda !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants