Kdb rewrite base #4943

hannes99 · 2023-05-23T13:18:11Z

This is the "framework" for the new C cli.
I'll mention PRs for commands later here.

This can be merged as is, more commands can be added later.

~~add spec support for external commands~~: Kdb rewrite external #4951 (reviewed and now in kdb_rewrite_base)
~~add spec for C++ commands~~: Kdb rewrite cpp spec #4949 (reviewed and now in kdb_rewrite_base)
~~add set command~~: Kdb rewrite set #4954 (reviewed and now in kdb_rewrite_base)
add meta commands: Kdb rewrite meta(get, ls, rm, set, show) #4955
this one

Basics

Short descriptions of your changes are in the release notes
(added as entry in doc/news/_preparation_next_release.md which contains _(my name)_)
Please always add them to the release notes.
Details of what you changed are in commit messages
(first line should have module: short statement syntax)
References to issues, e.g. close #X, are in the commit messages.
The buildservers are happy. If not, fix in this order:
- add a line in doc/news/_preparation_next_release.md
- reformat the code with scripts/dev/reformat-all
- make all unit tests pass
- fix all memleaks
- fix the CI itself (or rebase if already fixed)
The PR is rebased with current master.

Checklist

I added unit tests for my code
I fully described what my PR does in the documentation
(not in the PR description)
I fixed all affected documentation (see Documentation Guidelines)
I fixed all affected decisions (see Decision Process)
I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
I updated all meta data (e.g. README.md of plugins and METADATA.ini)
I mentioned every code not directly written by me in reuse syntax

Review

Documentation is introductory, concise, good to read and describes everything what the PR does
Examples are well chosen and understandable
Code is conforming to our Coding Guidelines
APIs are conforming to our Design Guidelines
Code is consistent to our Design Decisions

Labels

Add the "work in progress" label if you do not want the PR to be reviewed yet.
Add the "ready to merge" label if everything is done and no further pushes are planned by you.

kodebach

I'm not a fan of using macros when you don't need to. I left some comments about that, but IMO we could also merge this with the macros as they are now, as long as they are better documented (especially hidden assumptions).

The reason looks all very good. I just have some minor suggestions.

I also had an idea how to improve the integration of the old C++ code, but again we can also leave it the way it is now, as long as it's documented properly. Specifically, about the external shell script commands, the docs should state whether that is supported only via C++ or even, if you replace cpp_main with a dummy so you can compile without C++.

doc/man/man1/kdb-get.1

src/tools/kdb/colors.c

src/tools/kdb/get.h

src/tools/kdb/main.c

src/tools/kdb/get.c

src/tools/kdb/command.h

src/tools/kdb/command.c

src/tools/kdb/colors.h

hannes99 · 2023-05-27T14:14:24Z

Thanks a lot for the feedback!

kodebach

Noticed a two minor inconsistencies/mistakes, please check. Otherwise, LGTM.

src/tools/kdb/command.h

src/tools/kdb/main.c

hannes99 · 2023-05-30T08:43:36Z

jenkins build libelektra please

kodebach

LGTM now, just some small improvements to the new docs.

src/tools/kdb/colors.h

src/tools/kdb/command.h

src/tools/kdb/external.c

hannes99 · 2023-05-31T17:00:04Z

jenkins build libelektra please

lukashartl

lgtm

doc/help/kdb-complete.md

markus2330

Tutorials are much better now, still quite a few formatting problems etc. (I only marked a few exemplary, please check for others yourself.)

doc/dev/kdb-add-new-command.md

doc/tutorials/external-commands.md

doc/news/_preparation_next_release.md

hannes99 · 2023-06-16T08:08:37Z

Thanks a lot for the feedback @markus2330 !

src/libs/elektra/errors.c

and add command.h containing helpful macros and a struct def that is needed later. Commands not implemented yet will be handles by the old implementation.

... to command.c for resolving bookmarks in keynames.

... remove cpp implementation and add test for the get command

... and (partial) fix #3742, fix #4028, fix #1164

@kodebach

... based on @kodebach 's feedback

@markus2330

... based on @markus2330 's feedback

hannes99 assigned kodebach and lukashartl and unassigned kodebach and lukashartl May 23, 2023

hannes99 requested review from kodebach and lukashartl May 23, 2023 13:18

hannes99 force-pushed the kdb_rewrite_base branch 2 times, most recently from edf8782 to 93485a3 Compare May 23, 2023 13:42

kodebach requested changes May 23, 2023

View reviewed changes

hannes99 force-pushed the kdb_rewrite_base branch 2 times, most recently from 4bd6d67 to 53f9bb0 Compare May 27, 2023 14:06

hannes99 force-pushed the kdb_rewrite_base branch 2 times, most recently from b51edcd to 026b455 Compare May 27, 2023 16:39

kodebach requested changes May 27, 2023

View reviewed changes

src/tools/kdb/command.h Show resolved Hide resolved

src/tools/kdb/command.h Outdated Show resolved Hide resolved

hannes99 force-pushed the kdb_rewrite_base branch from 026b455 to 813f9c7 Compare May 28, 2023 10:07

kodebach reviewed May 28, 2023

View reviewed changes

src/tools/kdb/command.h Show resolved Hide resolved

kodebach mentioned this pull request May 28, 2023

libelektra-opts error codes #4953

Open

kodebach reviewed May 28, 2023

View reviewed changes

src/tools/kdb/main.c Outdated Show resolved Hide resolved

hannes99 force-pushed the kdb_rewrite_base branch 2 times, most recently from c00b8cb to ef566b6 Compare May 30, 2023 03:47

kodebach approved these changes May 30, 2023

View reviewed changes

hannes99 force-pushed the kdb_rewrite_base branch 3 times, most recently from 499db7d to f14de05 Compare May 31, 2023 15:10

github-advanced-security bot found potential problems May 31, 2023

View reviewed changes

src/tools/kdb/external.c Fixed Show fixed Hide fixed

lukashartl approved these changes May 31, 2023

View reviewed changes

doc/help/kdb-complete.md Show resolved Hide resolved

hannes99 force-pushed the kdb_rewrite_base branch from 21193e3 to f14de05 Compare June 1, 2023 12:12

markus2330 requested changes Jun 14, 2023

View reviewed changes

github-advanced-security bot found potential problems Jun 16, 2023

View reviewed changes

src/libs/elektra/errors.c Dismissed Show dismissed Hide dismissed

hannes99 requested a review from markus2330 June 19, 2023 07:49

hannes99 and others added 25 commits July 4, 2023 11:53

kdb-cli: add new main.c

8b39708

and add command.h containing helpful macros and a struct def that is needed later. Commands not implemented yet will be handles by the old implementation.

kdb-cli: add helper functions

386b8ea

... to command.c for resolving bookmarks in keynames.

shell_recorder: add support for C cli warning/error format

4f1c366

kdb-cli: rewrite get command

1561c10

... remove cpp implementation and add test for the get command

kdb cli: add spec for c++ commands

e252d68

CI: add line plugin to debian-bullseye-full-xdg build

c96162a

kdb cli: add support for external commands, with and without spec

00df54b

kdb-cli: rewrite set

0616860

... and (partial) fix #3742, fix #4028, fix #1164

kdb: fix newline problem for record commands

0c97078

kdb: add more extensive help message for command misuse

833ddc4

doc: add tutorial for implementing a new command for kdb in C

e05ca3d

Restyled by prettier-markdown

c938a25

doc: add missing link

8b284a6

doc: opts: add that help message is now always generated

4673c72

doc: improve add-new-command tutorial

5ee6a1c

... based on @kodebach 's feedback

kdb: fix memory leak when printing help message

0ef3013

Restyled by clang-format

e21f235

Restyled by prettier-markdown

d9b4eeb

kdb: move GET_OPTION and GET_OPTION_KEY to command.h

d90d6f1

lib/kdb: add ELEKTRA_CLI_ERROR category

29906e1

remove old merge-command leftovers

515a13c

doc: fix formatting/spelling problems

7053e6b

... based on @markus2330 's feedback

fixup! lib/kdb: add ELEKTRA_CLI_ERROR category

6233506

kdb: add test for external command specification

fbcc5a1

kdb: add mountOdbc spec

fe6ba2e

hannes99 force-pushed the kdb_rewrite_base branch from 0f7b532 to fe6ba2e Compare July 5, 2023 00:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kdb rewrite base #4943

Kdb rewrite base #4943

hannes99 commented May 23, 2023 •

edited

kodebach left a comment

hannes99 commented May 27, 2023

kodebach left a comment •

edited

hannes99 commented May 30, 2023

kodebach left a comment

hannes99 commented May 31, 2023

lukashartl left a comment

markus2330 left a comment

hannes99 commented Jun 16, 2023

Kdb rewrite base #4943

Are you sure you want to change the base?

Kdb rewrite base #4943

Conversation

hannes99 commented May 23, 2023 • edited

Basics

Checklist

Review

Labels

kodebach left a comment

Choose a reason for hiding this comment

hannes99 commented May 27, 2023

kodebach left a comment • edited

Choose a reason for hiding this comment

hannes99 commented May 30, 2023

kodebach left a comment

Choose a reason for hiding this comment

hannes99 commented May 31, 2023

lukashartl left a comment

Choose a reason for hiding this comment

markus2330 left a comment

Choose a reason for hiding this comment

hannes99 commented Jun 16, 2023

hannes99 commented May 23, 2023 •

edited

kodebach left a comment •

edited