Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

keyRel2 #826

Merged
merged 2 commits into from
Jul 21, 2016
Merged

keyRel2 #826

merged 2 commits into from
Jul 21, 2016

Conversation

tom-wa
Copy link
Contributor

@tom-wa tom-wa commented Jul 10, 2016

Key relation types: Below, DirectBelow, Silbling, and Nephew
DirectBelow and Silbling are boolean tests
relation suffixes:
SN: the namespaces have to match
IN: ignoring namespaces completely
CN: cascading namespace matches all namespaces

currently returns:
-1 for invalid keys / usage error
0 for failed tests
1 if a boolean relation test succeeded

0 if non-boolean relation tests succeeded - the value is the number of levels the check key is below

@tom-wa tom-wa changed the title keyRel2 Draft: keyRel2 Jul 10, 2016

typedef enum
{
BSN, //Below Same Namespace, cascading namespace matches only cascading namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

should be in proposal first. And please suffix the names.

@tom-wa
Copy link
Contributor Author

tom-wa commented Jul 13, 2016

just wanted to know if it's basically ok (return values/relationship checks/etc) before i finish it up
any suggestions for other checks ?

@markus2330
Copy link
Contributor

markus2330 commented Jul 13, 2016

Its always good if you write what you want to know directly in the PR description :-) Again, its hard to say what you wanted to achieve. Please put proper API docu in the source.

The description of the return values seem to be flawed, -1 is used for two different purposes. Why do you think is the isSame test so important? I would rather go for -1: usage error, 0: failed, 1 succeeded (or even >1 for numbers and not only boolean tests)

And I need to get used to the enum values... Maybe a bit longer names would be better.

(comment was edited multiple times)

{
if (!key || !check) return -1;
if (!key->key || !check->key) return -1;
if (!keyCmp (key, check)) return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would avoid this, users can check with keyCmp themselves.

@tom-wa
Copy link
Contributor Author

tom-wa commented Jul 13, 2016

you are right about the return values. not even sure if checking if the keys are the same is needed here.
and 0 for failed check and dropping the -1 would better anyway i guess.

@tom-wa
Copy link
Contributor Author

tom-wa commented Jul 19, 2016

where should i put the testcases for keyRel2 ?

@tom-wa
Copy link
Contributor Author

tom-wa commented Jul 19, 2016

nvm, just found test_proposal.c

i've updated it now, and used longer names for the enum values

@@ -64,5 +186,9 @@ int main (int argc, char ** argv)
test_ksPopAtCursor ();
test_ksToArray ();

test_keyAsCascading ();
Copy link
Contributor

Choose a reason for hiding this comment

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

please run code formatter

@tom-wa tom-wa changed the title Draft: keyRel2 keyRel2 Jul 20, 2016
@tom-wa
Copy link
Contributor Author

tom-wa commented Jul 20, 2016

@markus2330 done

@markus2330 markus2330 merged commit 1d9d5a8 into ElektraInitiative:master Jul 21, 2016
@markus2330
Copy link
Contributor

Thank you, well done!

@markus2330 markus2330 mentioned this pull request Jul 30, 2016
@tom-wa tom-wa deleted the keyRel2 branch January 30, 2017 02:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants