12
167
Fork
You've already forked fnott
32

config: make section name parsing modular #24

Manually merged
dnkl merged 1 commit from :modular-config-section-patch into master 2021年08月25日 10:41:58 +02:00
Contributor
Copy link

Export section parsing related structures to the top level, and add a function
to convert from a section name to the corresponding section enum. This
allows the section name matching to be modular, via calling the conversion
function on the scanned key instead of chained if statements. It also allows
global properties to be specified to be declared under the "main" section.

Export section parsing related structures to the top level, and add a function to convert from a section name to the corresponding section enum. This allows the section name matching to be modular, via calling the conversion function on the scanned key instead of chained if statements. It also allows global properties to be specified to be declared under the "main" section.
dnkl left a comment
Copy link

I think getting rid of the chained if-statements is an improvement.

But I don't see much reason to move the section definitions out of the parse_config_file() function; that's the only place they are used, and I like to keep variable scope as tight as possible. (str_to_section() is small enough that I don't see any reason it can't be inline:d in parse_config_file()).

Or do you have more work planned, that depend on this?

I think getting rid of the chained `if`-statements is an improvement. But I don't see much reason to move the section definitions out of the `parse_config_file()` function; that's the only place they are used, and I like to keep variable scope as tight as possible. (`str_to_section()` is small enough that I don't see any reason it can't be inline:d in `parse_config_file()`). Or do you have more work planned, that depend on this?
Author
Contributor
Copy link

You are right, there isn't any point to move the section definitions out to the top level, I basically modelled the config parsing block from foot. I will work on the requested changes.

No, at least not in the short term. I am not super comfortable with C and mainly made this PR to make the section parsing logic more modular as well as to make it easier for the configuration to be serialized to other formats without additional logic for global properties(it is a direct consequence of the first change).

Sorry for my bad english.

You are right, there isn't any point to move the section definitions out to the top level, I basically modelled the config parsing block from foot. I will work on the requested changes. No, at least not in the short term. I am not super comfortable with C and mainly made this PR to make the section parsing logic more modular as well as to make it easier for the configuration to be serialized to other formats without additional logic for global properties(it is a direct consequence of the first change). Sorry for my bad english.
polykernel force-pushed modular-config-section-patch from 6d0cc943d6 to 4540dde327 2021年08月24日 17:24:09 +02:00 Compare
polykernel force-pushed modular-config-section-patch from 4540dde327 to 9ddd7c8c31 2021年08月24日 17:28:29 +02:00 Compare
dnkl left a comment
Copy link

LGTM!

LGTM!
dnkl manually merged commit 6efbc04e3e into master 2021年08月25日 10:41:58 +02:00
Owner
Copy link

Thanks!

Thanks!
polykernel deleted branch modular-config-section-patch 2021年08月25日 16:44:32 +02:00
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dnkl/fnott!24
Reference in a new issue
dnkl/fnott
No description provided.
Delete branch ":modular-config-section-patch"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?