Oh and... +1 :) Thanks, Eduard
On Wed, May 30, 2012 at 5:01 PM, Eduard Moraru <[email protected]> wrote: > > > On Wed, May 30, 2012 at 4:16 PM, Thomas Mortagne < > [email protected]> wrote: > >> On Wed, May 30, 2012 at 2:55 PM, Eduard Moraru <[email protected]> >> wrote: >> > I mentioned Velocity because I saw a DiffScriptService component with >> > @Named("diff") that has the same method signature as the Java API: >> > >> > public <E> DiffResult<E> diff(List<E> previous, List<E> next, >> > DiffConfiguration<E> configuration) >> > >> > See the discussion [1] on the pull request for more details on the >> Velocity >> > question. >> > >> > Back to my original question (same question as Thomas basically), that's >> > exactly what I am trying to understand, Vincent... if the >> > List<String/Character> is the optimal use case and not just String. >> > >> > Without a scenario in favour of List<E>, I would go for exposing a >> String >> > based API (well, just an extra method in this case) as well. >> >> You can imagine all sort of use cases with any value so I don't see >> the point in arbitrary limiting the API to just Strings where the >> logic is the same whatever the type. > > > Yes, I understand now how it works. If other are interested, please see my > new comment [1] on the pull request. > > Short version: The API compares lists of objects by using Object#equals() > and does not go into the structure of the compared elements, just reports > differences of the list itself. You have to structure/break your input to > the level of granularity that you need for the diff to be relevant to your > use case. > > >> If you really need a direct use >> case for existing stuff then just think about producing a diff between >> two static list properties values, it would be very bad to have to >> create a String from it first especially since you could have \n in >> one of the elements. >> >> My main concern is to expose a clear API and no String it everything >> but clear, it's simple yes but it's not the same things. When you >> provide String you don't have no idea what you are diffing exactly >> (could be characters, \n, words, etc.) so having only one method with >> two strings is just very bad design. >> >> Also if you read my mail I did talked about String based APIs but as >> helpers and I'm would be surely -1 for any attempt to restrict the >> whole API to it. So the only question here is: is that really needed >> to expose String based helpers or not. >> > > No, I was never suggesting to drop the generic API, I was just saying that > the helpers might be a good idea since I imagine that String diff would > cover most of the usage of this API. Anyway, this is not important for now > since it is something that can be added later if API clients start > complaining :) > > Thanks, > Eduard > > ---------- > [1] https://github.com/xwiki/xwiki-commons/pull/2/files#r900063 > > >> > >> > Thanks, >> > Eduard >> > >> > ---------- >> > [1] https://github.com/xwiki/xwiki-commons/pull/2/files#L27R58 >> > >> > On Wed, May 30, 2012 at 3:32 PM, Vincent Massol <[email protected]> >> wrote: >> > >> >> >> >> On May 30, 2012, at 2:28 PM, Vincent Massol wrote: >> >> >> >> > >> >> > On May 30, 2012, at 2:18 PM, Eduard Moraru wrote: >> >> > >> >> >> Hi Thomas, >> >> >> >> >> >> On Tue, May 29, 2012 at 5:07 PM, Thomas Mortagne >> >> >> <[email protected]>wrote: >> >> >> >> >> >>> Hi devs, >> >> >>> >> >> >>> As I said in another mail I'm working on a diff/merge module to >> use in >> >> >>> XWiki, the first target being Extension Manager and document >> history >> >> >>> (since both are going to use the same code, see Marius mails). >> >> >>> >> >> >>> You can see the detail on >> >> https://github.com/xwiki/xwiki-commons/pull/2. >> >> >>> >> >> >>> TODO/QUESTIONS: >> >> >>> * improve the generic 3 ways merge to be at least as good as the >> >> >>> List<String> 3 ways merge and get rid of JDiff >> >> >>> * not sure where/if I should put String related helpers (things >> that >> >> >>> do a diff on two String instead of having to cut List<String> or >> >> >>> List<Character> and call the diff API etc.) >> >> >>> >> >> >> >> >> >> This looks a bit awkward to use, specially from (but not limited to) >> >> >> Velocity. When I think of a diff service API, I imagine passing >> Strings, >> >> >> not lists of composing strings or list of characters. >> >> > >> >> > Thomas is proposing a Java API here AFAIK. For Velocity we never >> expose >> >> java API directly; we use Script Services for that. >> >> >> >> Sorry, just realized you wrote "but not limited to" ;) >> >> >> >> I'll let Thomas reply since I don't know this diff api yet…. >> >> >> >> Sorry for the noise >> >> -Vincent >> >> >> >> > >> >> > Thanks >> >> > -Vincent >> >> > >> >> > PS: FTR I'm a big -1 to model our Java APIs to be easily usable from >> >> Velocity since that means having suboptimal APIs for the wrong reason >> ;) >> >> > >> >> >> Also, I don`t quite understand the List<String> approach. I can >> imagine >> >> the >> >> >> List<Character> version by breaking a string into characters, but >> what >> >> >> would you do for List<String>? You would split a String using "\s" >> as a >> >> >> separator? >> >> >> >> >> >> Can you please mention a use case where a list oriented API is >> better >> >> than >> >> >> a string based one? >> >> >> >> >> >> Thanks, >> >> >> Eduard >> >> >> >> >> >>> >> >> >>> I'm ready to merge it into master so I'm waiting your vote. >> >> >>> >> >> >>> Caleb is it OK to put new stuff in master already or should I wait >> the >> >> >>> complete release to be done ? >> >> >>> >> >> >>> Here is my +1. >> >> >>> >> >> >>> Thanks, >> >> >>> -- >> >> >>> Thomas Mortagne >> >> >> >> _______________________________________________ >> >> devs mailing list >> >> [email protected] >> >> http://lists.xwiki.org/mailman/listinfo/devs >> >> >> > _______________________________________________ >> > devs mailing list >> > [email protected] >> > http://lists.xwiki.org/mailman/listinfo/devs >> >> >> >> -- >> Thomas Mortagne >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs >> > > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
