-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fix/issue 33 #34
Fix/issue 33 #34
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tarun-kavipurapu thanks for the fix, can you please add some tests around this.
@tarun-kavipurapu can you please check for |
@tarun-kavipurapu please resolve the conflicts. |
|
||
// command2callback map | ||
var command2callback = map[string]DiceDBCallback{ | ||
"AUTH": renderSimpleString, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this other way around? Like group methods to commands list?
It'd reduce redundancy.
Something like below and add a finder on top of this
// Create hashsets (maps) for each command group
var simpleStringCommands = map[string]struct{}{
"AUTH": {}, "PING": {}, "SELECT": {},
"SET": {}, "RESTORE": {}, "FLUSHDB": {},
}
strItem := fmt.Sprintf("%v", item) | ||
|
||
// Properly format with double quotes | ||
quoted := fmt.Sprintf("\"%q\"", strItem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it just be single quotes? We're printing with 2 quotes? Is behaviour same as CLI?
dice > KEYS k*
1) ""k""
2) ""k1""
internal/db/dicedb.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests.
Closing, as not required after recent SDK changes. Thanks a lot for the efforts @tarun-kavipurapu . |
Fixes #33