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

Features/diff generator #184

Merged
merged 7 commits into from
Mar 4, 2013
Merged

Features/diff generator #184

merged 7 commits into from
Mar 4, 2013

Conversation

cholin
Copy link

@cholin cholin commented Mar 4, 2013

generator implementation for diff (#183)

@arrbee
Copy link
Member

arrbee commented Mar 4, 2013

@cholin How did that patch part of the diff API work for you? Was is documented clearly enough? It's still relatively new, so I'm happy for any feedback.

For example, I've considered adding an API that let's you run linearly over the lines in the patch instead of requiring nested loops, a la:

typedef struct {
    git_diff_range range;
    const char *header;
    size_t header_len;
    size_t hunk_of_patch;
    size_t lines_in_hunk;
} git_diff_hunk;

typedef struct {
    char line_origin;
    const char *content;
    size_t content_len;
    size_t line_of_hunk;
    int old_lineno;
    int new_lineno;
} git_diff_line;

GIT_EXTERN(size_t) git_diff_patch_num_lines(
    const git_diff_patch *patch);

GIT_EXTERN(int) git_diff_patch_get_line(
    const git_diff_hunk **hunk_out,
    const git_diff_line **line_out,
    const git_diff_patch *patch,
    size_t line);

You could look at the line_of_hunk field in the line object to know when you were starting a new hunk (and the hunk_out pointer had changed).

Anyhow, I don't know that this is necessarily a good idea, but since you've been using the API recently and since it hasn't seen much use yet, I thought I'd try to solicit some feedback. Thanks!

@cholin
Copy link
Author

cholin commented Mar 4, 2013

@arrbee: Documentation was good! I found all the necessary informations through the api documentation. I will give you a more detailed feedback in a few days. Currently I'm short on time. ;)

@cholin
Copy link
Author

cholin commented Mar 4, 2013

@arrbee: Ok I missed the bus, so here are my thoughts about diff in libgit2

The documentation in the diff.h is really good but unfortunately all constants are missing at libgit2.github.com/libgit2 (like GIT_DELTA_* or GIT_DIFF__). For git_diff_find_similiar I had problems with too small files (error msg: *File too small for similarity signature calculation_). The message is clear but I had to dig deep into the code to get the minimum size to do a similarity signature calculation. If I'm right it's 127 (defined in hashsig.c - HASHSIG_HEAP_SIZE). As well I thought that the check aborts with this error msg ( but that was just a feeling ;) ).

I think your consideration about extending the diff api to run linearly over the lines is a good idea. With that it's really easy to get all the informations in a flat iteration style. But I don't know if it's perhaps confusing for the user
with all these possiblities to get the diff informations.

Overall I think you did a great job with this api! 👍

@jdavid jdavid merged commit 86b063f into libgit2:master Mar 4, 2013
@cholin cholin deleted the features/diff_generator branch March 4, 2013 21:44
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.

3 participants