Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt: add group by metadata #613

Merged
merged 1 commit into from
Apr 29, 2023
Merged

opt: add group by metadata #613

merged 1 commit into from
Apr 29, 2023

Conversation

xiaoyifang
Copy link
Owner

close #612

@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch 3 times, most recently from 01ee26c to 6b0eada Compare April 29, 2023 07:45
@shenlebantongying
Copy link
Collaborator

shenlebantongying commented Apr 29, 2023

If we implement more features on metadata.txt, then we will have to maintain it.

The format will be locked unless we want to break someone and got hated.

If it ever got expanded, we will reinvent a config format.

This will happen because renaming will also likely be implemented here.

Maybe we should consider using existing human-oriented formats like toml https://toml.io/ so that we don't need to maintain/write related parsing code?

Of course, it is not hard, but like the custom command line parser, there are details like report errors on typos / write-to-config that requires unnecessary work. Also, we don't need to describe the syntax, just link to https://toml.io/

But, so far I can only think about 4 possible fields in future 😅

Category = [ "english", "yellow", "green" ]

[metadata]
name = "New Name"
langfrom = "English"
langto = "Russian"

@xiaoyifang
Copy link
Owner Author

Maybe we should consider using existing human-oriented formats like toml https://toml.io/ so that we don't need to maintain/write related parsing code?

are there some c++ library which can use to parse the toml format ?

@shenlebantongying
Copy link
Collaborator

shenlebantongying commented Apr 29, 2023

How about this: https://github.com/marzer/tomlplusplus

We just need to include a single file into the source tree https://github.com/marzer/tomlplusplus/blob/master/toml.hpp

or

git submodule add --depth 1 https://github.com/marzer/tomlplusplus.git thirdparty/tomlplusplus

# in qmake
INCLUDEPATH += thirdparty/tomlplusplus/include

# in cmake

target_include_directories(${CMAKE_PROJECT_NAME} PUBLIC
        ${PROJECT_SOURCE_DIR}/thirdparty/tomlplusplus/include

@xiaoyifang
Copy link
Owner Author

Great, I'll see what I can do .

@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch from 6b0eada to 4ad6cf2 Compare April 29, 2023 10:06
@xiaoyifang
Copy link
Owner Author

I'm really not comfortable/familiar with this modern syntax of tomlplus 😿

@shenlebantongying
Copy link
Collaborator

You seems forget to add the submodule,

@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch 2 times, most recently from 6b72655 to 99c5e3b Compare April 29, 2023 10:31
@shenlebantongying
Copy link
Collaborator

shenlebantongying commented Apr 29, 2023

btw, toml++ includes 3 formatters (toml, yml & json:sweat_smile: ), if we don't need them we can

DEFINES += TOML_ENABLE_FORMATTERS=0

target_compile_definitions(${CMAKE_PROJECT_NAME} PUBLIC TOML_ENABLE_FORMATTERS=0)  

marzer/tomlplusplus#158

@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch from 99c5e3b to 3f19632 Compare April 29, 2023 11:04
@xiaoyifang
Copy link
Owner Author

btw, toml++ includes 3 formatters (toml, yml & json😅 ), if we don't need them we can

DEFINES += TOML_ENABLE_FORMATTERS=0

target_compile_definitions(${CMAKE_PROJECT_NAME} PUBLIC TOML_ENABLE_FORMATTERS=0)  

marzer/tomlplusplus#158

the author said that this will not effect performance and size.

@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch 2 times, most recently from 10ef5fd to a353fbf Compare April 29, 2023 11:25
@shenlebantongying
Copy link
Collaborator

I tested with some invalid inputs, and the code works OK to me :D

@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch from a353fbf to 9579f37 Compare April 29, 2023 11:31
@xiaoyifang xiaoyifang force-pushed the fix/issue-group-metadata branch from 9579f37 to 1f20e1d Compare April 29, 2023 12:07
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 4 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 451 Code Smells

0.0% 0.0% Coverage
2.9% 2.9% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

group by metadata implementation
2 participants