widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing

>  Is there a better way? A hint on that is welcome :-)

printf debugging is what I use most of the times too - pdb is not a great debugger. however I have a snippet in my editor that will format all lines I output like so: print "#sirver: a: %s" % a.

Similarly if i note something that I want to fix in a branch before submitting a merge proposal, I add a comment: NOCOM(#sirver). Before sending stuff for review, I grep for '#sirver' in the code base - if there are none left I know i am done.

Diff comments:

> === modified file 'mainpage/templatetags/wl_markdown.py'
> --- mainpage/templatetags/wl_markdown.py	2014-12-23 16:59:05 +0000
> +++ mainpage/templatetags/wl_markdown.py	2015-01-07 14:22:19 +0000
> @@ -55,12 +55,16 @@
>  def _insert_smileys( text ):
>      """
>      This searches for smiley symbols in the current text
> -    and replaces them with the correct images
> +    and replaces them with the correct images.
> +    Only replacing if smiley symbols aren't in a word (e.g. http://....)
>      """
> +    words = text.split(" ")
>      for sc,img in SMILEYS:
> -        text = text.replace(sc,"<img src='%s%s' alt='%s' />" % ( SMILEY_DIR, img, img ))
> +        if sc in words:
> +            words[words.index(sc)] = "<img src='%s%s' alt='%s' />" % ( SMILEY_DIR, img, img )
> +    text = " ".join(words)
> +    return text
>  
> -    return text
>  def _insert_smiley_preescaping( text ):
>      """
>      This searches for smiley symbols in the current text
> @@ -140,6 +144,7 @@
>  
>  def do_wl_markdown( value, *args, **keyw ):
>      # Do Preescaping for markdown, so that some things stay intact
> +    # This is currently only needed for this smiley ">:-)"
>      value = _insert_smiley_preescaping( value )
>  
>      custom = keyw.pop('custom', True)
> @@ -154,7 +159,7 @@
>          # well, empty soup. Return it
>          return unicode(soup)
>  
> -    ctag = soup.contents[0]
> +    #ctag = soup.contents[0]

Remove or uncomment. Do not check in commented lines of code please. That is what source control is for.

>  
>      for text in soup.findAll(text=True):
>          # Do not replace inside a link
> @@ -167,8 +172,10 @@
>          if custom:
>              # Replace bzr revisions
>              rv = _insert_revision( text )
> -            # Replace smileys
> -            rv = _insert_smileys( rv )
> +            # Replace smileys; only outside "code-tags"
> +            if not text.parent.name == "code":
> +                rv = _insert_smileys( rv )
> +                #b
>  
>              for name, (pattern,replacement) in custom_filters.iteritems():
>                  if not len(text.strip()) or not keyw.get(name, True):
> @@ -182,7 +189,7 @@
>      # The function goes from .5 ms to 1.5ms on my system
>      # Well, for our site with it's little traffic it's maybe not so important...
>      soup = BeautifulSoup(unicode(soup)) # What a waste of cycles :(
> -
> +    #b
>      # We have to go over this to classify links
>      
>      for tag in soup.findAll("a"):
> 
> === modified file 'media/css/base.css'
> --- media/css/base.css	2014-12-23 16:59:05 +0000
> +++ media/css/base.css	2015-01-07 14:22:19 +0000
> @@ -69,8 +69,25 @@
>  	display: inline;
>  }
>  
> +/**************/
> +/* Codeblocks */
> +/**************/
> +
>  pre {
>  	white-space: pre-wrap;
> +	background-image: url("../img/black50.png");
> +	padding: 5px 10px;
> +	margin: 5px 10px;
> +	display: inline-block;
> +	border: 1px solid black;
> +}
> +
> +code {
> +	background-image: url("../img/black50.png");
> +}
> +/*No double background*/
> +pre > code {
> +	background-image: none;
>  }
>  
>  input, button, textarea, .button {
> @@ -199,10 +216,6 @@
>  	margin: 0px 20px;
>  }
>  
> -div#header {
> -
> -}
> -
>  div #main {
>  	min-height: 500px;
>  }
> @@ -225,7 +238,9 @@
>  	border-radius: 4px;
>  }
>  
> +/**********/
>  /* Header */
> +/**********/
>  
>  div#header  img{
>  	position: relative;
> @@ -280,7 +295,9 @@
>  	color: #ffffff;
>  }
>  
> +/****************/
>  /* Right Column */
> +/****************/
>  
>  div#rightColumn {
>  	width: 220px;
> @@ -418,3 +435,4 @@
>  	border: none;
>  	clear: left;
>  }
> +
> 
> === modified file 'media/css/wiki.css'
> --- media/css/wiki.css	2014-12-20 18:49:15 +0000
> +++ media/css/wiki.css	2015-01-07 14:22:19 +0000
> @@ -2,7 +2,7 @@
>  	border: 1px solid black;
>  	display: inline-block;
>  	padding: 0px 8px;
> -	background-image: url("../img/black50.png");
> +	background-image: url("../img/black20.png");
>  	font-size: 12px;
>  	margin: 0em 0em 1em 1em;
>  	line-height: 18px;
> 
> === modified file 'pybb/util.py'
> --- pybb/util.py	2012-04-19 20:21:28 +0000
> +++ pybb/util.py	2015-01-07 14:22:19 +0000
> @@ -174,7 +174,8 @@
>  
>      # if markup == 'markdown':
>      if markup == 'markdown':
> -        return '>'+text.replace('\r','').replace('\n','\n>') + '\n'
> +        #Adding a space after ">" will kepp some things stay in tact

kepp -> keep. or rather reword:

"Inserting a space after ">" will not change the generated HTML, but it will unbreak certain constructs like 'insert an example here'.

> +        return '> '+text.replace('\r','').replace('\n','\n> ') + '\n'
>      elif markup == 'bbcode':
>          return '[quote]\n%s\n[/quote]\n' % text
>      else:
> 
> === modified file 'settings.py'
> --- settings.py	2013-06-30 12:53:49 +0000
> +++ settings.py	2015-01-07 14:22:19 +0000
> @@ -139,14 +139,18 @@
>      ("O:-)", "face-angel.png"),
>      ("O:)", "face-angel.png"),
>      (":-/", "face-confused.png"),
> -    #(":/", "face-confused.png"),
> +    (":/", "face-confused.png"),
>      ("B-)", "face-cool.png"),
>      ("B)", "face-cool.png"),
>      (":'-(", "face-crying.png"),
>      (":'(", "face-crying.png"),
> +    (":-))", "face-smile-big.png"),
> +    (":))", "face-smile-big.png"),
> +    (":-)", "face-smile.png"),
> +    (":)", "face-smile.png"),
>      ("&gt;:-)", "face-devilish.png"), # Hack around markdown replacement. see also SMILEY_PREESCAPING
>      ("8-)", "face-glasses.png"),
> -    #("8)", "face-glasses.png"), # Might occur unwanted
> +    ("8)", "face-glasses.png"), # Might occur unwanted (why?)

Then remove the comment since it is no longer true.

>      (":-D", "face-grin.png"),
>      (":D", "face-grin.png"),
>      (":-x", "face-kiss.png"),
> @@ -164,10 +168,6 @@
>      (":(", "face-sad.png"),
>      (":-O", "face-shock.png"),
>      (":O", "face-shock.png"),
> -    (":-)", "face-smile.png"),
> -    (":)", "face-smile.png"),
> -    (":-))", "face-smile-big.png"),
> -    (":))", "face-smile-big.png"),
>      (":-o", "face-surprise.png"),
>      (":o", "face-surprise.png"),
>      (":-P", "face-tongue.png"),
> @@ -180,7 +180,7 @@
>  # This needs to be done to keep some stuff hidden from markdown
>  SMILEY_PREESCAPING = [
>      (">:-)", "\>:-)"),
> -    (":-*", ":-\*"),
> +    #(":-*", ":-\*"),

remove commented lines

>      #(":*", ":\*"),
>  ]
>  
> 


-- 
https://code.launchpad.net/~franku/widelands-website/smiley-and-codeblock-enhancements/+merge/245742
Your team Widelands Developers is subscribed to branch lp:widelands-website.