-
Notifications
You must be signed in to change notification settings - Fork 98
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
Added text difference finder #25
Conversation
Thanks! I'll take a look at this asap. (maybe after I publish my next video this weekend) |
Hi, FYI, I just added the MIT license to this project. Just wanted to let you know about it since this pull request might be the first major PR that's not made by me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry I thought I'd submitted this already.
Can you change this so that we split the strings by characters instead of by words?
Some languages are hard to split by words, for example Japanese.
|
||
<head> | ||
<meta charset="UTF-8"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, just curious, where did you get all these meta tags? Maybe you used a template?
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<meta http-equiv="X-UA-Compatible" content="ie=edge"> | ||
<title>Document</title> | ||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/materialize/1.0.0/css/materialize.min.css"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting that you have Materialize here. Why not Bootstrap? Or do people use them at the same time usually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I can change it to split by character, but I think for English it might be better just to split by words. Maybe we can split by words for alphabet-based languages like English and split by character for character-based languages like Japanese.
Also, the metatags are from the default template created by VSCode (text editor).
I used Materialize just for the demo since I like it a bit better but you can definitely change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that might be a good idea. People might write mixed things though, so that might be more tricky.
Okay cool, Materialize sounds good then!
Okay, I might change the function to my version later, but the whole layout of this PR is really helpful. So, I'll merge it for now and probably change the implementation a bit later. |
I used the longest common subsequence algorithm to create a text difference finder. I added the Javascript and CSS into a global static folder, which I registered in settings.py.
The text difference finder is in
text_difference.js
and can be called with the functionfindTextDifference()
. The function returns an array of 3 strings: one that shows the deletions and additions, one that shows only the deletions, and one that shows only the additions. Each string is formatted using a span element with either a "deleted" or "added" class.