[Shadow][RFC] add CliRequest class

Review Request #87 — Created April 23, 2013 and discarded

olivier
Lunr
https://github.com/owizen/lunr/tree/CliRequest
7315
lunr
Add the CliRequest class which makes use of the GetoptCliParser

This review depends on http://reviews.lunr.nl/r/76/ from Heinz


  • 1
  • 0
  • 2
  • 0
  • 3
Description From Last Updated
Needs to be compatible to the normal Request class. Don't know what's the best course of action here... pprkut pprkut
olivier
pprkut
  1. 
      
  2. src/Lunr/Shadow/CliRequest.php (Diff revision 1)
     
     
    Needs to be compatible to the normal Request class. Don't know what's the best course of action here...
    1. This is the big problem because we should then uniformize calls to post, get, json "arrays" and the array of the command line.
      as a fallback I can always backport the class to skyline in case we need more experience about this special case.
    2. What's a "skyline"?
      Apart from that I don't get the whole uniformation you dream about. Care to explain?
  3. src/Lunr/Shadow/CliRequest.php (Diff revision 1)
     
     
    Dependency Injection
    1. I was hesitating but if you say so...
      we can choose between inject the ast or the parser...
      Let me know. for this precise case I did not see any problem in not injecting.
    2. No object instantiation in the constructor. Think about what works best
  4. src/Lunr/Shadow/CliRequest.php (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    what's the proposed use-case for this?
    1. So first, the get_option_count method can be removed if needed.
      
      For the use case I based my tests(basic script using your wrap_argument method) on the get_opt page from the php manual.
      http://www.php.net/manual/fr/function.getopt.php
      
      
      1)look at the example 2 (with use of 3 --required):
      
      The following command is executed:
      
      >php cli.php  -f "value for f" -v -a --required value1 --required value2 --required value3 --optional="optional value" --option
      
      #get_opt returns:
      
      array(6) 
      {
        ["f"]       => string(11) "value for f"
        ["v"]       => bool(false)
        ["a"]       => bool(false)
        ["required"]=> array(3){[0]=>string(6) "value1"  [1]=>string(6) "value2"  [2]=>string(6) "value3"}
        ["optional"]=> string(14) "optional value"
        ["option"]  => bool(false)
      }
      
      #GetoptCliParser::parse returns:
      
      array(6) 
      {
        ["f"]       => array(1) { [0]=>string(11) "value for f" }
        ["v"]       => array(0) {}
        ["a"]       => array(0) {}
        ["required"]=> array(1) { [0]=>array(3) {[0]=>string(6) "value1" [1]=> string(6) "value2" [2]=>string(6) "value3" } }
        ["optional"]=> array(1) { [0]=>string(14) "optional value" }
        ["option"]  => array(0) {}
      }
      
      
      2)look at the example 3:
      
      The following command is executed:
      
      >php cli.php  -aaac
      
      #get_opt returns:
      
      array(2) 
      {
        ["a"]=> array(3) {[0]=> bool(false) [1]=> bool(false) [2]=> bool(false) }
        ["c"]=> bool(false)
      }
      
      
      GetoptCliParser::parse returns:
      
      array(2) 
      {
        ["a"]=> array(1) { [0]=> array(3) { [0]=> bool(false) [1]=> bool(false) [2]=> bool(false) } }
        ["c"]=> array(0) {}
      }
      
      So the use case is 
      
      $request = $new CliRequest($short, $long);
      
      $value = $request->get_argument('a');
      $a_count = $request->get_option_count('a');
      
    2. The output of parse is made to be compatible between cli parser. So no matter whether you handle a getopt ast or a lunr cli ast, it will be the same.
      I don't get though what that has to do with anything. Think about how this class will be used. What is necessary to be retrieved from the request class?
      If you just write wrapper functions around the ast what's the point of having the wrapper functions?
  5. 
      
olivier
olivier
olivier
olivier
Review request changed

Status: Discarded

Change Summary:

We have the mailing list for discussing how to implement something. Come back here when you have something tangible (preferably including unit tests)
Loading...