lukasschwab / stackit

StackOverflow queries from the command line
MIT License
314 stars 28 forks source link

Refactoring #9

Closed WnP closed 9 years ago

WnP commented 9 years ago

I've made a lot of change in the source code to make it more pythonic (pep8 included), readable and maintenable

If you have some question let me know

WnP commented 9 years ago

I think some explanations can be useful:

lukasschwab commented 9 years ago

@WnP you're killing it!

I'll check this out, and will hopefully have it merged by EOD. Thanks!

schwartzmx commented 9 years ago

@lukasschwab @WnP This looks really good, now its starting to look like some clean python code. The reason for that ugly try, except stuff was to just band aid the problem temporarily. But a PR like this was something I had in mind in the near future for this project, good job @WnP! Looks great.

Also if you are interested @WnP, have a look at a discussion we were having a few days ago. And chime in if you'd like. Since this is taken care of, next I want to look into this problem and where stackit should head with this.

lukasschwab commented 9 years ago

@WnP I encountered an error dropping back from the question focus view into the list view. The problem arises because the list view is generated starting at the selected question upon return––e.g., if I'm looking at the list view for questions 1-5, select 4, and then enter x, the resulting listview will try to return questions 4-9.

There isn't appropriate handling for cases where the script attempts to print more questions than exist in the list. This is not an issue, I believe, that existed before these changes.

Here's the CLI interaction, with some abridgments marked by ...

$ python stackit_core.py -s 'test'
Searching for: test...
Tags: 
1
Question: Why is processing a sorted array faster than an unsorted array?
Answer** You are the victim of [ branch prediction ](//en.wikipedia.org/wiki/Branch_predictor) fail. **

...

4
Question: Checkout remote Git branch
Answer

[ Jakub's answer ](http://stackoverflow.com/a/1787014/456814) actually
improves on this. With Git versions ≥ 1.6.6, you can just do:

5
Question: Avoiding "!= null" statements in Java?
Answer

This to me sounds like a reasonably common problem that junior to intermediate
developers tend to face at some point: they either don't know or don't trust
the contracts they are participating in and defensively overcheck for nulls.
Additionally, when writing their own code, they tend to rely on returning
nulls to indicate something thus requiring the caller to check for nulls.

Enter m for more, a question number to select, or q to quit: 4
-------------------------QUESTION------------------------
Checkout remote Git branch

I am trying to checkout a remote branch:

Somebody pushed a branch called ` test ` with ` git push origin test ` to a
shared repository. I can see the branch with ` git branch -r ` . But how can I
get this branch?

  * ` git checkout test ` does nothing 

  * ` git checkout origin/test ` does something, but ` git branch ` says ` * (no branch) ` . I am on no branch? 

How do I share branches via a public repository?

-------------------------------ANSWER------------------------------------

##  Update

[ Jakub's answer ](http://stackoverflow.com/a/1787014/456814) actually
improves on this. With Git versions ≥ 1.6.6, you can just do:

      git fetch
      git checkout test

(User masukomi points out below that ` git checkout test ` will NOT work in
modern git if you have multiple remotes which have the same branch name, git
doesn't know which one to use. In this case use ` git checkout -b test remote-
name/test ` )

##  Old Answer

Before you can start working locally on a remote branch, you need to fetch it
as called out in answers below.

To fetch a branch, you simply need to:

       git fetch origin

This will fetch all of the remote branches for you. You can see the branches
available for checkout with:

       git branch -v -a

With the remote branches in hand, you now need to check out the branch you are
interested in, giving you a local working copy:

       git checkout -b test origin/test

Enter b to launch browser, x to return to search, or q to quit: x

4
Question: Checkout remote Git branch
Answer

[ Jakub's answer ](http://stackoverflow.com/a/1787014/456814) actually
improves on this. With Git versions ≥ 1.6.6, you can just do:

5
Question: Avoiding "!= null" statements in Java?
Answer

This to me sounds like a reasonably common problem that junior to intermediate
developers tend to face at some point: they either don't know or don't trust
the contracts they are participating in and defensively overcheck for nulls.
Additionally, when writing their own code, they tend to rely on returning
nulls to indicate something thus requiring the caller to check for nulls.

Traceback (most recent call last):
  File "stackit/stackit_core.py", line 197, in <module>
    main()
  File "/Library/Python/2.7/site-packages/click/core.py", line 610, in __call__
    return self.main(*args, **kwargs)
  File "/Library/Python/2.7/site-packages/click/core.py", line 590, in main
    rv = self.invoke(ctx)
  File "/Library/Python/2.7/site-packages/click/core.py", line 782, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Library/Python/2.7/site-packages/click/core.py", line 416, in invoke
    return callback(*args, **kwargs)
  File "/Library/Python/2.7/site-packages/click/decorators.py", line 64, in new_func
    return ctx.invoke(f, obj, *args[1:], **kwargs)
  File "/Library/Python/2.7/site-packages/click/core.py", line 416, in invoke
    return callback(*args, **kwargs)
  File "stackit/stackit_core.py", line 192, in main
    _search(config)
  File "stackit/stackit_core.py", line 106, in _search
    focus_question(question_logs)
  File "stackit/stackit_core.py", line 79, in focus_question
    select(questions, int(user_input))
  File "stackit/stackit_core.py", line 62, in select
    print_question(questions[j], j + 1)
IndexError: list index out of range
WnP commented 9 years ago

@lukasschwab this - aaed221 - fix the bug on listing, nevertheless it no longer list questions 4 to 9 after a selecting question 4, now it returns questions 1 to 5

I think it's possible to return 4 to 9, but it required some more change which I guess are out of this PR

I've fixed another bug on print_question function, which was asserting an error when the answer is only a code block -so no p tag- this is commented in code

@schwartzmx I'm going to read that thread and give you some feedback, thanks ;-)

lukasschwab commented 9 years ago

Did some testing, this looks great! To avoid this kind of confusion in the future, it would be great to separate refactoring from functionality changes––makes testing easier, as functionality changes in refactoring would normally raise a red flag.

I'm going to move forward with a merge and a new release.

WnP commented 9 years ago

yes, I agree that normally I should made another PR for this functionality

thanks for merging

WnP commented 9 years ago

@lukasschwab the gif demo in README.md is no longer up to date, because help message have change and is not shown if you just run stackit, you now need to run it using -h or --help to see it

maybe we should show a message that tell the user it must read the usage using -h or --help when no argument are passed to stackit

WnP commented 9 years ago

@lukasschwab you could find an up to date gif here

if it fits your needs

lukasschwab commented 9 years ago

@WnP you're solving problems as fast as I can add them to my to-do list!

Adding to README.md

WnP commented 9 years ago

Sorry ^^

Thx