Skip to content

Add support for .syn synonym files.#20

Merged
Dushistov merged 2 commits intoDushistov:masterfrom
ecraven:master
Jul 6, 2017
Merged

Add support for .syn synonym files.#20
Dushistov merged 2 commits intoDushistov:masterfrom
ecraven:master

Conversation

@ecraven
Copy link
Copy Markdown
Contributor

@ecraven ecraven commented Jul 6, 2017

Fixes #8.

@Dushistov
Copy link
Copy Markdown
Owner

Can you add some simple dictionary with .syn somewhere to tests/ and
some script to check that sdcv handle .syn in right way?

@ecraven
Copy link
Copy Markdown
Contributor Author

ecraven commented Jul 6, 2017

ok, added three tests, seems to work fine, I tried with some larger local dictionaries too ;)

Copy link
Copy Markdown
Owner

@Dushistov Dushistov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for update, it need little fixes for merge.

tmpstr = (gchar *)g_memdup(p2, p3-p2+1);
tmpstr[p3-p2] = '\0';
syn_wordcount = atol(tmpstr);
g_free(tmpstr);
Copy link
Copy Markdown
Owner

@Dushistov Dushistov Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplify parsing of integers, so to fix build you need:

syn_wordcount = atol(std::string(p2, p3-p2).c_str());

tests/t_synonyms Outdated

unset SDCV_PAGER

RES=$($SDCV -n --data-dir "$TEST_DIR" foo | grep result)
Copy link
Copy Markdown
Owner

@Dushistov Dushistov Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could move repeated pattern into sh function, for example:

function test_word() {
    WORD="$1"
    echo "Test ${WORD}"
    RES=$($SDCV -n -u "Test synonyms" --data-dir "$TEST_DIR" "${WORD}" | grep result)
    if [ "result" != "$RES" ]; then
        echo "synonym for "${WORD}" should be result but was \"${RES}\"" >&2
        exit 1
    fi
}

test_word "foo"
test_word "bar"
test_word "test"

also to make possible code to run on machine with dictionaries with "test" word into it,
please add -u "Test synonyms" into sdcv arguments

@ecraven ecraven force-pushed the master branch 2 times, most recently from cc601f6 to 29b4d83 Compare July 6, 2017 17:50
@ecraven
Copy link
Copy Markdown
Contributor Author

ecraven commented Jul 6, 2017

I hope I have addressed all the changes.

@Dushistov Dushistov merged commit f510300 into Dushistov:master Jul 6, 2017
@Dushistov
Copy link
Copy Markdown
Owner

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants