Move static methods out of HTMLPage

[uranusjr]

#5800 part 1b-1 (part of #5822).

All static and class methods are moved out of HTMLPage, preparing for HTMLPage’s removal. The diff is large because HTMLPage.get_page() was a huge function, so I did only copy-pasting in this PR, and intentionally kept all the functions as identical as possible. I have already prepared a refactor for _get_html_page(), but this should be much more straightforward to review.

[cjerdonek]

When it comes to the ordering of classes and functions in a module, etc, how do you feel about following the rule (when possible) that functions should be defined before (aka "above") where they are used? That way modules can be read top to bottom (or phrased another way, more specific to general, building on what's defined above).

Even though this issue does come up in a small way in this PR, I'm asking this question more for the future because I expect it will come up a lot in your future refactorings. Otherwise, I think this looks good.

[uranusjr]

I agree! It is also a rule I generally follow when I implement a thing from scratch. I’ve been organising code in three sections [stuff to keep]-[stuff extracted from HTMLPage]-[HTMLPage] for this series of refactoring, to avoid moving things too wildly, so the diff could be easier to read. For this particular set though, I guess the diff would be as easy to read if I put _get_html_page() (and functions it uses) after HTMLPage instead. I’ll modify this a bit.

[cjerdonek]

Great! Oh, I was thinking to put your pulled out functions at the top since they are pretty focused..

…

On Sat, Sep 29, 2018 at 10:41 PM Tzu-ping Chung ***@***.***> wrote: I agree! It is also a rule I generally follow when I implement a thing from scratch. I’ve been organising code in three sections [stuff to keep]-[stuff extracted from HTMLPage]-[HTMLPage] for this series of refactoring, to avoid moving things too wildly, so the diff could be easier to read. For this particular set though, I guess the diff would be as easy to read if I put _get_html_page() (and functions it uses) after HTMLPage instead. I’ll modify this a bit. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5833 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVt7skyZF8uu2YANN510lErP8L-L9I_ks5ugFl7gaJpZM4XAqYp> .

[uranusjr]

Ah, yeah, that would make sense. I think I’ll do this in another upcoming PR, when I remove HTMLPage altogether. Otherwise it would generate a pretty useless diff on the whole HTMLPage class.

[cjerdonek]

Sorry, let me explain more clearly. I was suggesting putting only your pulled out functions at the top and leaving everything else the same, so the diff would be the same number of lines. The thinking is that your pulled out functions should go in the right place from the beginning, since it is "go-forward" code. Otherwise, you'll need to move them a second time again. The HTMLPage class you can leave where it is in this commit (even though it's breaking the rule), because it's not changing and you're going to be doing more with it later anyways (namely removing it). Does that make more sense?

[uranusjr]

So the order would be

1. Extracted functions (including _get_html_page())
2. PackageFinder and egg_info_matches (not changed)
3. HTMLPage

is this correct? It makes sense to me.

[cjerdonek]

Yes, thanks!

[uranusjr]

GitHub messed the commit ordering, but here’s the edited and rebased changeset.

[cjerdonek]

Okay, great. Happy to merge this now, though @pradyunsg said to hold off on merging things until after the release this week, so I'll wait until after that.

[cjerdonek]

@pradyunsg is there an appropriate topic label for the PR's that @uranusjr is working on in this area? I didn't see one for "index" or something like that.

[pradyunsg]

Feel free to merge this and to add a new label for things related to package finder / index etc.

I wanted to be on top of PRs that merge just between now and the release basically.

[cjerdonek]

Thanks again, @uranusjr!

Also, @uranusjr, since you've been putting the most thought into this, do you have any suggestions for the name and description of a new label to describe this topic area (e.g. "finder" or "index," etc)? Here is the current list: https://github.com/pypa/pip/issues/labels

[uranusjr]

I feel “finder” (or “package finder” if that’s not too long) would be the most appropriate. The module being named index has confused me a little in the beginning, since it handles more than indexes.