- 
                Notifications
    You must be signed in to change notification settings 
- Fork 196
Enable override default headers from CLI #205
Conversation
        
          
                hyper/cli.py
              
                Outdated
          
        
      | headers[i.key] = i.value | ||
| # :key:value case | ||
| if i.key == '': | ||
| k, v = i.value.split(':') | 
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.
While I don't think it's likely we could get a header with a colon in it (the only possible one is :path, which might be able to have a colon in it), we should defend against that anyway. Let's swap this to i.value.split(':', 1).
| Fantastic, everything is green! I have a few minor style notes that I'd like you to change, if possible, but when that's done I think this will be ready to merge. | 
| I changed this block as you suggested. | 
        
          
                hyper/cli.py
              
                Outdated
          
        
      | else: | ||
| # when overriding a HTTP/2 special header there will be a leading | ||
| # colon, which tricks the command line parser into thinking | ||
| # the header is empty | 
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.
Mind indenting this one more level?
| Sorry, I wasn't quite clear enough on the indentation. Fix that up and we're all good! =) | 
| Magnificent, thank you so much @pkrolikowski! ✨ 🍰 ✨ | 
Enable override default headers from CLI
| I am glad I could help :) So what do you think, what could I do next ? | 
@Lukasa I had a problem with push to rebased branch, so I decided to make a new request.