Open ishepard opened 7 years ago
Not sure if I understood. Modification has a getDiff()
already. What
exactly do you want there?
On Tue, Sep 19, 2017, 13:44 Spadini Davide notifications@github.com wrote:
In org.repodriller.domain.Modification.java there are getAdded() and getRemoved(), methods that return the number of added and removed lines.
However, there is no way to get the actual lines that are added and removed. Indeed, this is done in other classes (DiffParser and DiffBlock).
Isn't it better to add these method in the Modification.java instead?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mauricioaniche/repodriller/issues/62, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGJzGrAl1GO2jls5vEsXxK8Je-eqFFTks5sj7cogaJpZM4PcVXm .
-- Maurício Aniche Postdoc researcher Delft University of Technology www.mauricioaniche.com @mauricioaniche
I would like to get a list of the Added/Removed lines, directly from the Modification.
Something like Modification.getAddedLines()
Davide Spadini | PhD student +31 06 11 41 19 55 | d.spadini@sig.eu mailto:d.spadini@sig.eu Software Improvement Group | www.sig.eu http://www.sig.eu/
On 19 Sep 2017, at 15:57, Maurício Aniche notifications@github.com wrote:
Not sure if I understood. Modification has a
getDiff()
already. What exactly do you want there?On Tue, Sep 19, 2017, 13:44 Spadini Davide notifications@github.com wrote:
In org.repodriller.domain.Modification.java there are getAdded() and getRemoved(), methods that return the number of added and removed lines.
However, there is no way to get the actual lines that are added and removed. Indeed, this is done in other classes (DiffParser and DiffBlock).
Isn't it better to add these method in the Modification.java instead?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mauricioaniche/repodriller/issues/62, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGJzGrAl1GO2jls5vEsXxK8Je-eqFFTks5sj7cogaJpZM4PcVXm .
-- Maurício Aniche Postdoc researcher Delft University of Technology www.mauricioaniche.com @mauricioaniche — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mauricioaniche/repodriller/issues/62#issuecomment-330547282, or mute the thread https://github.com/notifications/unsubscribe-auth/AItqtRMhMl6iJy2UZ3y-fNLC-HeNUQyCks5sj8gugaJpZM4PcVXm.
Looking through here, I'm a bit confused about why Modification doesn't make use of the various Diff classes.
It would make more sense to me if a Modification returned your choice of the raw diff or a list of DiffBlock
s as available via DiffParser
. If it returned the DiffBlocks
then extracting the added/removed lines would become the problem of a DiffBlock.
In general I think it makes sense to minimize the use of raw diffs -- parsing them into a more powerful class as done by DiffParser
seems like the way to go.
@mauricioaniche It's hard for me to understand the current design. Can you help out?
DiffParser
indeed currently looks like a utility helper. No reason for that. We can definitely make Modification
to return the more elegant diff objects provided by DiffParser
.
But I guess what Davide wanted in this issue was a method that would return only the added lines. However, using the Diff objects, one can easily write a lambda for that (using the DiffLineType
).
Yes, that's what I had in mind. If a Modification can return something semantically relevant instead of a raw diff, then the caller can easily extract the added and removed lines. I don't think Modification itself should need to do that.
@ishepard Would this meet your needs?
Yes indeed this is exactly what I wanted.
DiffParser and DiffBlock classes are hidden in the middle of repodriller. In my opinion, Modification should not return the raw diff, but it should use this helper classes to return a more elegant object. This would help a lot the life of miners ;)
@mauricioaniche This would be a breaking change. Thoughts?
We should not remove the return the raw diff as that would be a breaking change, indeed. However, we can add new methods to Modification
that encapsulates the new DiffParser(..).parse()
, so that users can easily find this feature. New methods are welcome!
@ayaankazerouni Did you do anything for this?
In
org.repodriller.domain.Modification.java
there aregetAdded()
andgetRemoved()
, methods that return the number of added and removed lines.However, there is no way to get the actual lines that are added and removed. Indeed, this is done in other classes (
DiffParser
andDiffBlock
).Isn't it better to add these method in the
Modification.java
instead?