loewensteinph / google-diff-match-patch

Automatically exported from code.google.com/p/google-diff-match-patch
Apache License 2.0
0 stars 0 forks source link

"Word Mode" - The definition of a 'word' #48

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
---What steps will reproduce the problem?
1. Try implementing the "Word Mode" explained here 
http://code.google.com/p/google-diff-match-patch/wiki/LineOrWordDiffs
2. Diff some stuff
3. Eat pizza

---What is the expected output? What do you see instead?
We should have the choice of determining what is a "word". Does it include the 
delimiter (whitespace) before it? Does it include the delimiter following it? 
Is the delimiter itself considered a separate 'word' in its own right?
Instead, the decision seems to have been hardcoded that the word-delimiter 
(whitespace in my case) is part of the previous word.

---What version of the product are you using? On what operating system?
JavaScript version
Windows XP
Testing with IE8 and Chrome

---Please provide any additional information below.
I need every word-delimiter (whitespace in my case) to be considered a separate 
'word' in its own right. Any quick fixes to do this please?

Original issue reported on code.google.com by logic....@gmail.com on 19 May 2011 at 4:33

GoogleCodeExporter commented 8 years ago
This is why DMP doesn't have a built-in word mode; no two people can agree on 
what a word is, what to do with punctuation, etc.  By writing your own function 
as the linked page describes, you get to define your own delimiters.

To implement your desired behaviour, you might want something like this in your 
diff_linesToWords function:

if (text.charAt(0).match(/\s/) {
  // The next token will be whitespace.
  lineEnd = text.indexOf(/\S/, lineStart);
} else {
  // The next token will be a word.
  lineEnd = text.indexOf(/\s/, lineStart);
}

Original comment by neil.fra...@gmail.com on 19 May 2011 at 5:54

GoogleCodeExporter commented 8 years ago
Oops, I forgot that text isn't being chopped at each iteration.  That first 
line should be:

if (text.charAt(lineStart).match(/\s/) {

Original comment by neil.fra...@gmail.com on 19 May 2011 at 5:57

GoogleCodeExporter commented 8 years ago
Currently working on something else but I will get back to my issue and try 
this out sometime soon. Thanks a lot for the help, very much appreciated!

Original comment by logic....@gmail.com on 23 May 2011 at 9:11

GoogleCodeExporter commented 8 years ago
I'm trying to use this but am not getting desired results...

1. First off, I don't think indexOf() takes regex, not according to 
specification anyway apparently. I am using a custom "regexIndexOf()" function 
i found online, here it is:
===CODE===
String.prototype.regexIndexOf = function(regex, startpos) {
 var indexOf = this.substring(startpos || 0).search(regex);
 return (indexOf >= 0) ? (indexOf + (startpos || 0)) : indexOf;
}
===/CODE===

2. It doesn't seem to work, I still see whitespace (in my case ' ' space 
character) considered part of its adjacent previous word. To illustrate, 
consider this string "Hello world people"
I need DMP to diff it by words as follows:
[Hello] [ ] [world] [ ] [people]
Instead, I get:
[Hello ] [world ] [people]

Thanks in advance for your time if you get around to this... :)

Original comment by logic....@gmail.com on 2 Jun 2011 at 10:04

GoogleCodeExporter commented 8 years ago
*
(when I said "it doesn't seem to work" I am referring to your snippet you 
posted above, including your follow-up correction)

Original comment by logic....@gmail.com on 2 Jun 2011 at 10:05

GoogleCodeExporter commented 8 years ago
Hm... Why not just Add string/array with word delimiters and enum such like:

Enum TreatWord
{
DelimeterOnStart
DelimeterOnEnd
DelimeterOnBothSides
}

And depending on this make diff for words. This will cover all situations "on 
what a word is".

By the way, the linked example is obsolete and for adding word diff 
functionality you have to change two methods as I can remember.

Original comment by g33.ad...@gmail.com on 25 Jun 2011 at 11:46