range plugin #1334

tom-wa · 2017-02-09T06:58:48Z

Purpose

range plugin preview

Checklist

commit messages are fine (with references to issues)
I ran all tests and everything went fine
I added unit tests
affected documentation is fixed
I added code comments, logging, and assertions
meta data is updated (e.g. README.md of plugins)

markus2330

Some refactoring would be good and the usual things are missing (docu, comments). But looks nice for a first draft!

markus2330 · 2017-02-09T10:37:04Z

src/plugins/range/range.c

+			{
+				pos = 1;
+				char * endPtr;
+				a = strtoll (ptr, &endPtr, 10);


should we move this code to a base lib? It is so common..

i don't think there's much gain in it, but probably the way the current version handles it (union + enum) ?

markus2330 · 2017-02-09T10:37:37Z

src/plugins/range/testmod_range.c

+
+	init (argc, argv);
+
+	test ("5", 1, "1-10");


nice test code!

markus2330

small comments.

markus2330 · 2017-02-09T15:56:44Z

src/plugins/range/README.md

 ## Dependencies ##

 None.

 ## Examples ##

-None.
+```sh
+# Backup-and-Restore:/examples/range


Please make sure to add the test in tests/shell/shell_recorder/tutorial_wrapper/CMakeLists.txt or, (even better) let us automatically include all plugins README.md to be checked with the shell recorder!

markus2330 · 2017-02-09T15:57:48Z

src/plugins/range/README.md

-You can use `scripts/copy-range`
-to automatically rename everything to your
-plugin name:
+The plugin checks every Key in the Keyset for the Metakey `check/range` which contains either a single range with the syntax `[-]min-[-]max`, or a list of ranges separated by `,` and tests if the Key value is within the range(s).


Are spaces allowed? Please do not make lines too long.

markus2330

Thank you, nicely done! Please rebase and refactor a bit.

markus2330 · 2017-03-01T21:17:25Z

doc/METADATA.ini

+[check/range/type]
+status= implemented
+usedby/plugin= range
+description= type of range (INT, FLOAT, HEX or CHAR)


Why not use check/type and type instead?

markus2330 · 2017-03-01T21:17:38Z

src/error/specification

@@ -1033,3 +1033,17 @@ severity:error
 ingroup:plugin
 module:date
 macro:D_T_FMT
+


needs rebase

markus2330 · 2017-03-01T21:18:05Z

src/plugins/README.md

@@ -202,6 +202,7 @@ copied by another plugin just before):
 - [conditionals](conditionals/) by using if-then-else like statements
 - [required](required/) rejects non-required keys
 - [date](date/) validates date and time data
+- [range](range/) checks if a value is within a specific range


specific -> given

markus2330 · 2017-03-01T21:18:39Z

src/plugins/range/README.md

+
+## Usage ##
+
+The plugin checks every Key in the Keyset for the Metakey `check/range` which contains either a single range with the syntax `[-]min-[-]max`, or a list of ranges separated by `,` and tests if the Key value is within the range(s).


Is it also possible to specify 3,5,7?

markus2330 · 2017-03-01T21:19:13Z

src/plugins/range/README.md

+
+The plugin checks every Key in the Keyset for the Metakey `check/range` which contains either a single range with the syntax `[-]min-[-]max`, or a list of ranges separated by `,` and tests if the Key value is within the range(s).
+
+`check/range/type` can be used to specify the datatype. If not specified otherwise the default value is `INT`


Please use type/check/type here, otherwise this is a unnecessary pitfall.

(For hex we need an extra meta data, see discussion #1388)

Internally, you can use INT,.. but please not in the user interface.

markus2330 · 2017-03-01T21:23:37Z

src/plugins/range/README.md

+
+# should succeed
+kdb set /examples/range/value 5
+kdb setmeta user/examples/range/value check/range "1-10"


why here a namespace but not at other places? Why not writing it to spec?

markus2330 · 2017-03-01T21:24:20Z

src/plugins/range/range.c

+typedef struct
+{
+	RangeType type;
+	union _Value {


Names starting with _ are reserved.

markus2330 · 2017-03-01T21:24:49Z

src/plugins/range/range.c

+
+static int rangeStringToRange (const char * rangeString, RangeValue * min, RangeValue * max, RangeType type)
+{
+	int factorA = 1;


What is factorA and B? Please comment!

markus2330 · 2017-03-01T21:25:55Z

src/plugins/range/range.c

+	max.type = type;
+	int rc = rangeStringToRange (rangeString, &min, &max, type);
+	/*
+		switch (type)


functions are getting too long

move to (commented out) helper function.

markus2330 · 2017-03-01T21:31:47Z

src/plugins/range/range.c

+	long long int tmpIB = factorB * b.Value.i;
+	long double tmpFA = factorA * a.Value.f;
+	long double tmpFB = factorB * b.Value.f;
+	switch (type)


really long function...

markus2330 · 2017-03-05T16:45:21Z

Thank you, great job! For HEX we another decision is pending..

tom-wa added the work in progress label Feb 9, 2017

markus2330 reviewed Feb 9, 2017

View reviewed changes

tom-wa changed the title ~~range: initial commit~~ range plugin Feb 9, 2017

markus2330 reviewed Feb 9, 2017

View reviewed changes

tom-wa added ready to merge and removed work in progress labels Feb 28, 2017

tom-wa force-pushed the range branch 2 times, most recently from 7084487 to 71fabb9 Compare February 28, 2017 06:38

tom-wa mentioned this pull request Feb 28, 2017

typedispatcher #1373

Closed

6 tasks

markus2330 suggested changes Mar 1, 2017

View reviewed changes

markus2330 removed the ready to merge label Mar 3, 2017

tom-wa force-pushed the range branch from d64cf2b to 9ffa137 Compare March 5, 2017 13:58

tom-wa added 15 commits March 5, 2017 14:59

range: initial commit

2bd12c3

range: better parsing

03b24c4

range: added float support

71f54f7

range: added hex support

b86430e

range: fixed datatypes

922a8b4

range: added plugin to README.md

82ec1a5

range: errors and README.md

892b194

range: added metadata to README.md

dc8a4b0

range: added char support

db6bdc3

range: removed debug output and unused arguments

e47875c

range: reformat source

cc59520

range: added to METADATA.ini

9e93f41

range: export validateKey and remove unused functions

1e93ed9

range: fixed METADATA.ini

a943ec1

range: refactor, use type / check/type

9ffa137

tom-wa force-pushed the range branch from 9b8383c to 9f0b973 Compare March 5, 2017 14:03

METADATA.ini: add range to check/type

d2eadd0

tom-wa force-pushed the range branch from 9f0b973 to 3674ee6 Compare March 5, 2017 14:05

range: update metadata in README.md

3674ee6

tom-wa added the ready to merge label Mar 5, 2017

markus2330 merged commit bc8054d into ElektraInitiative:master Mar 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

range plugin #1334

range plugin #1334

tom-wa commented Feb 9, 2017 •

edited

Loading

markus2330 left a comment

markus2330 Feb 9, 2017

tom-wa Feb 16, 2017

markus2330 Feb 9, 2017

markus2330 left a comment

markus2330 Feb 9, 2017

markus2330 Feb 9, 2017

markus2330 left a comment

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 Mar 1, 2017

markus2330 commented Mar 5, 2017


		## Usage ##

		The plugin checks every Key in the Keyset for the Metakey `check/range` which contains either a single range with the syntax `[-]min-[-]max`, or a list of ranges separated by `,` and tests if the Key value is within the range(s).


		The plugin checks every Key in the Keyset for the Metakey `check/range` which contains either a single range with the syntax `[-]min-[-]max`, or a list of ranges separated by `,` and tests if the Key value is within the range(s).

		`check/range/type` can be used to specify the datatype. If not specified otherwise the default value is `INT`

range plugin #1334

range plugin #1334

Conversation

tom-wa commented Feb 9, 2017 • edited Loading

Purpose

Checklist

markus2330 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 commented Mar 5, 2017

tom-wa commented Feb 9, 2017 •

edited

Loading