You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/10/08 14:28:23 UTC

[GitHub] [incubator-kvrocks] tanruixiang opened a new pull request, #963: (WIP) Add parse util function

tanruixiang opened a new pull request, #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963

   For commands with many variable parameters, the current parsing method is still relatively rigid.  In order to uniformly parse this multi-optional parameter type command, a util function is added.
   ```
   Status ParseCommandSyntax(const std::vector<std::string> &args,
                               const std::unordered_map<std::string, int> &key_words,
                               std::unordered_map<std::string, std::vector<std::string>> *result)
   ```
   `args` stores the command that needs to be parsed. 
   
   `key_words` indicates the keyword need to parse the command and the number of parameters required. For example, the number `ex` needs to parse is 2: the ex and the `ex's value`.
   
   `result`  stores the value parsed by the `key_words`. For example, what `ex` parses is an array with a length of 1 and the array's only value is `ex's value`.
   
   it close #962 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272340815

   > > I think the function generally looks fine, but it seems we now need a well-designed parsing framework rather than some non-generic util functions now.
   > > I am making some attempts at this, although I may not be able to devote particularly much time to it these days.
   > 
   > I agree. If we need to implement a very generalized function, a single util function is not enough.
   > 
   > Maybe you can split your work into some issues for everyone to implement together?
   
   Of cause, if everything goes well I will come up with some design ideas and plan out some implementation tasks in the next week or so.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tanruixiang commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
tanruixiang commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272340334

   > I think the function generally looks fine, but it seems we now need a well-designed parsing framework rather than some non-generic util functions now.
   > 
   > I am making some attempts at this, although I may not be able to devote particularly much time to it these days.
   
   I agree. If we need to implement a very generalized function, a single util function is not enough. 
   
   Maybe you can split your work into some issues for everyone to implement together?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun closed pull request #963: (WIP) Add parse util function(Waiting for parsing framework design)

Posted by GitBox <gi...@apache.org>.
tisonkun closed pull request #963: (WIP) Add parse util function(Waiting for parsing framework design)
URL: https://github.com/apache/incubator-kvrocks/pull/963


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272341894

   Another concern is that, there are so many string copy in both current code and this PR. Maybe we need to find a way to reduce them, and take advantages of move semantics.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] git-hulk commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272337005

   hmmm... It looks NOT generalized enough, I think the main is that has no good way to map the receiver name and type, so we can't know how to parse the field value.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on pull request #963: (WIP) Add parse util function(Waiting for parsing framework design)

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1287995602

   Since the code base changes a lot, I suggest you reimplement this patch if you're still interested on it.
   
   Closing...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272339644

   I think the function generally looks fine, but it seems we now need a well-designed parsing framework rather than some non-generic util functions now.
   
   I am making some attempts at this, although I may not be able to devote particularly much time to it these days.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] caipengbo commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
caipengbo commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272718336

   One idea I have is that we implement syntactic state machine for each command, and then use a utility function to parse the state machine.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tanruixiang commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
tanruixiang commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272338302

   > hmmm... It looks NOT generalized enough, I think the main is that has no good way to map the receiver name and type, so we can't know how to parse the field value.
   
   The main reason is that different command keywords and analysis methods are also different, so it is difficult to unify. Is there any more elegant way to do it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] maochongxin commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
maochongxin commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272341285

   I think it can be roughly classified into several types of parameters. Can it be classified by some features such as SFINAE?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tanruixiang commented on pull request #963: (WIP) Add parse util function

Posted by GitBox <gi...@apache.org>.
tanruixiang commented on PR #963:
URL: https://github.com/apache/incubator-kvrocks/pull/963#issuecomment-1272332660

   @git-hulk @PragmaTwice @torwig @caipengbo Do you have any better ideas? After discussing a suitable method, I will follow up the replacement work.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org