search

jessepeterson / margarita

Web frontend for reposado
The Unlicense
245 stars 40 forks source link

Description #19

Closed neographophobic closed 11 years ago

neographophobic commented 11 years ago

Enable the display of the update's description. It uses bootstraps modal window, so can be closed by clicking outside of the window, clocking the close buttons or hitting escape.

To enable the display of the description information, two additional python modules were needed, the first is BeautifulSoup, and the second is the lxml parser (used in conjunction with BeautifulSoup).

An example of how a description is displayed is as follows:- description

I've also added the update ID as a tool tip, and appended it to the description. This should assist people using the newer builds of Munki with Reposado and Margarita when applying pkginfos to Apple Updates.

neographophobic commented 11 years ago

This change addresses #7 and also #17.

neographophobic commented 11 years ago

lxml will require gcc to be installed on OS X. This can be installed from the XCode Command Line tools package.

jessepeterson commented 11 years ago

Thanks for the pull! While definitely in the plans to offer the description I'm wary of anything that requires a compiler or other heavy dependencies with Margarita. Originally had though of doing the parsing using the client side, but not totally sure yet.

neographophobic commented 11 years ago

My javascript sucks, but I've found a way to do this client side without any additional server side modifications, using just jQuery.

I see you have pushed a heap of changes to a mvc branch. I'm happy to submit the changes, and will base them off either head of master or mvc, which would you prefer?

jessepeterson commented 11 years ago

The mvc branch, once vetted and tested, will almost certainly be where future development goes, so that'd be preferred.

More generally though I was thinking the description functionality would be a part of the table view—such that one could expand/collapse multiple descriptions and have them in the list at once. However, perhaps a start where you've taken it is best.

jessepeterson commented 11 years ago

Having a closer look at this I think we can use some native Python to tear out the html, body, head tags and such to just include the p tags (as your pull request already does using BeautifulSoup). Something like the below which seems to work for all updates in my catalogs. Note in the interest of speed I tried to avoid using regexps, but not sure if that makes sense or how quick the difference would make.

I'm thinking of putting the output of this into a jQuery context (e.g. $('<div>' + descriptionHtml + '</div>')) and then insert it into the view somehow. Doing it dynamically I think would be important as the possibility of rogue HTML in the update descriptions could do bad things if the HTML is just directly put into the document.

def get_description_content(html):
    if len(html) == 0:
        return None

    # in the interest of (attempted) speed, try to avoid regexps
    lwrhtml = html.lower()

    celem = 'p'
    startloc = lwrhtml.find('<' + celem + '>')

    if startloc == -1:
        startloc = lwrhtml.find('<' + celem + ' ')

    if startloc == -1:
        celem = 'body'
        startloc = lwrhtml.find('<' + celem)

        if startloc != -1:
            startloc += 6 # length of <body>

    if startloc == -1:
        # no <p> nor <body> tags. bail.
        return None

    endloc = lwrhtml.rfind('</' + celem + '>')

    if endloc == -1:
        endloc = len(html)
    elif celem != 'body':
        # if the element is a body tag, then don't include it.
        # DOM parsing will just ignore it anyway
        endloc += len(celem) + 3

    return html[startloc:endloc]

As you may have seen also, I merged the mvc branch into master. BackboneJS and Marionette it is going forward.

neographophobic commented 11 years ago

Hi,

I've got it working client side with jQuery, but it will take me a little bit to clean up my patches in light of your recent changes. It's too slow to do all records, so I have it doing the parsing of the HTML (for p, ul, and ol tags) on demand, which has the added benefit of not wrecking the layout.

I'll hopefully get a chance to submit the patch tomorrow or the day after.

Cheers, Adam

On 13/03/2013, at 7:41 PM, Jesse Peterson wrote:

Having a closer look at this I think we can use some native Python to tear out the html, body, head tags and such to just include the p tags (as your pull request already does using BeautifulSoup). Something like the below which seems to work for all updates in my catalogs. Note in the interest of speed I tried to avoid using regexps, but not sure if that makes sense or how quick the difference would make.

I'm thinking of putting the output of this into a jQuery context (e.g. $('

' + descriptionHtml + '
')) and then insert it into the view somehow. Doing it dynamically I think would be important as the possibility of rogue HTML in the update descriptions could do bad things if the HTML is just directly put into the document.

def get_description_content(html): if len(html) == 0: return None

# in the interest of (attempted) speed, try to avoid regexps
lwrhtml = html.lower()

celem = 'p'
startloc = lwrhtml.find('<' + celem + '>')

if startloc == -1:
    startloc = lwrhtml.find('<' + celem + ' ')

if startloc == -1:
    celem = 'body'
    startloc = lwrhtml.find('<' + celem)

    if startloc != -1:
        startloc += 6 # length of <body>

if startloc == -1:
    # no <p> nor <body> tags. bail.
    return None

endloc = lwrhtml.rfind('</' + celem + '>')

if endloc == -1:
    endloc = len(html)
elif celem != 'body':
    # if the element is a body tag, then don't include it.
    # DOM parsing will just ignore it anyway
    endloc += len(celem) + 3

return html[startloc:endloc]

As you may have seen also, I merged the mvc branch into master. BackboneJS and Marionette it is going forward.

— Reply to this email directly or view it on GitHub.

neographophobic commented 11 years ago

It's going to take me way too long to pickup the new framework, and figure out how to use it to make this change.

I've pushed the changes to the previous branch. Hopefully you can translate this work into the new frameworks. Code is at https://github.com/neographophobic/margarita/tree/client_side_description

jessepeterson commented 11 years ago

Fantastic, thanks! This is the next priority in Margarita and so I'll be giving it time soon I hope (probably not until the weekend, though). I'll take a look at your branch!

jessepeterson commented 11 years ago

Hi Adam. Take a look at the recent commit. Sort of a WIP and am not too thrilled at how the grey colors turn out but this is what I was thinking for the info/description viewing.

jessepeterson commented 11 years ago

Closing this pull request. More than happy to talk about the new implementation. Thanks for your time and patches, though!