Custom Column Renaming #50
Conversation
|
Thanks for opening this PR, will take a look at it upcoming weekend and/or week! |
| m := regexp.MustCompile(`(?i)` + substr) | ||
| s = string(m.ReplaceAll([]byte(s), []byte(substr))) |
There was a problem hiding this comment.
Usually seeing a regex in a loop is a huge performance penalty - but since this is not a real-time API and "just" a CLI tool, lets leave it for now, we can improve anytime if we encounter real performance issues.
One change request tho, lets use s = m.ReplaceAllString(s, substr) - this way we can get rid of all the back and forth casting between bytes and strings.
| desc: "no rules", | ||
| input: "", | ||
| expected: nil, | ||
| }, |
There was a problem hiding this comment.
Lets add a test case without separator (invalid rule:value pair).
| const multiSplitter = "," | ||
| const ruleSplitter = ":" |
There was a problem hiding this comment.
I like but can also hard-code where it's used. If you decide to keep them here, what do you think about to rename and grouping these like
const (
rulesSeparator = ","
ruleSeparator = ":"
)|
|
||
| // ParseCustomRenameRules reads a custom rename setting string | ||
| // and returns a map for easier rule access. | ||
| // customRenameRule example: column1:Column2,col10:column10. |
There was a problem hiding this comment.
Lets be super explicit about this and mention the format first and then an example?
Format: from:to
Example: providerId:providerID
Multiple rules can be specified by comma: column1:Column2,col10:column10.
| m := make(map[string]string, strings.Count(settings.CustomColumnRename, multiSplitter)+1) | ||
| for _, rule := range strings.Split(settings.CustomColumnRename, multiSplitter) { |
There was a problem hiding this comment.
Still thinking about runtime performance here: lets split once and use the length of the rules:
rules := strings.Split(settings.CustomColumnRename, rulesSeparator)
m := make(map[string]string, len(rules))
for ...| @@ -49,6 +49,7 @@ func NewCmdArgs() (args *CmdArgs) { | |||
| flag.Var(&args.Null, "null", "representation of NULL columns: sql.Null* (sql) or primitive pointers (native|primitive)") | |||
|
|
|||
| flag.BoolVar(&args.NoInitialism, "no-initialism", args.NoInitialism, "disable the conversion to upper-case words in column names") | |||
| flag.StringVar(&args.CustomColumnRename, "ccr", args.CustomColumnRename, "add custom colum rename rules and ignore column renaming functions, skipping initialism and casing (format: providerId:ProviderID,123:abc)") | |||
There was a problem hiding this comment.
Same thought as in the comment of the godoc of the parsing function, lets be super explicit here, since this is the helper/usage output for users of this tool. Maybe something along the lines
use custom-column-rules to overwrite column naming and initialism. Format ...
Can you double check the typo colum -> column?
| @@ -308,19 +300,37 @@ func replaceSpace(r rune) rune { | |||
| return r | |||
| } | |||
|
|
|||
| // renameColumn changes the column name if the change rule | |||
| // in settings applied, otherwise returns original string and false. | |||
| func renameColumn(cfg *settings.Settings, s string) (string, bool) { | |||
| // renameColumn changes the column name if the change rule | ||
| // in settings applied, otherwise returns original string and false. | ||
| func renameColumn(cfg *settings.Settings, s string) (string, bool) { | ||
| m := cfg.ParseCustomRenameRules() |
|
Hey @Rhymond - is this feature still wanted? Do you think you can find some time in the next couple of weeks to address my comments? Thanks a lot! |
-ccr(custom column rename). It ignores any other formatting rules. Very useful when you have column for example:providerId, the usual process would rename it toProvIDerId, with custom rule defined (providerId:ProviderID) you can rename to expectedProviderID.