Commit c6ee2a48 authored by Grant Paton-Simpson's avatar Grant Paton-Simpson

Add regex advisors; misc

* Add regex advisors
* Fix code controlling display of snippet
* Put code for storing ast XML into separate function
* Bump version
parent 17b9443c
Pipeline #568 failed with stages
# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.9.6
version number: 0.9.7
author: Grant Paton-Simpson
## Overview
......
......@@ -2,7 +2,7 @@ from setuptools import setup, find_packages # @UnresolvedImport
from codecs import open
from os import path
__version__ = '0.9.6'
__version__ = '0.9.7'
here = path.abspath(path.dirname(__file__))
......
from ..advisors import snippet_advisor
from .. import conf
from ..utils import get_python_version, layout_comment as layout
def get_str_els_3_7(block_el):
str_els = block_el.xpath('descendant-or-self::Str')
return str_els
def get_str_val_3_7(val_el):
str_val = val_el.get('s')
return str_val
def get_str_els_3_8(block_el):
constant_els = block_el.xpath('descendant-or-self::Constant')
str_els = [constant_el for constant_el in constant_els
if constant_el.get('type') == 'str']
return str_els
def get_str_val_3_8(val_el):
str_val = val_el.get('value')
return str_val
python_version = get_python_version()
if python_version in (conf.PY3_6, conf.PY3_7):
get_str_els = get_str_els_3_7
get_str_val = get_str_val_3_7
elif python_version == conf.PY3_8:
get_str_els = get_str_els_3_8
get_str_val = get_str_val_3_8
else:
raise Exception(f"Unexpected Python version {python_version}")
def imported_re(block_el):
## straight import
import_els = block_el.xpath('descendant-or-self::Import/names/alias')
import_names = [el.get('name') for el in import_els]
if 're' in import_names:
return True
import_from_module_els = block_el.xpath('descendant-or-self::ImportFrom')
from_import_names = [el.get('module') for el in import_from_module_els]
if 're' in from_import_names:
return True
return False
def _imported_verbose_constant_attr(block_el):
attr_els = block_el.xpath('descendant-or-self::value/Attribute')
verbose_constant = False
for attr_el in attr_els:
if attr_el.get('attr') == conf.VERBOSE_FLAG:
verbose_constant = True
break
return verbose_constant
def _from_imported_verbose_constant(block_el):
import_from_els = block_el.xpath('descendant-or-self::ImportFrom')
verbose_constant = False
for import_from_el in import_from_els:
name_alias_els = import_from_el.xpath('descendant-or-self::names/alias')
for name_alias_el in name_alias_els:
if name_alias_el.get('name') == conf.VERBOSE_FLAG:
verbose_constant = True
break
return verbose_constant
def used_verbose_constant(block_el):
if _imported_verbose_constant_attr(block_el):
return True
if _from_imported_verbose_constant(block_el):
return True
return False
def used_inline_verbose(block_el):
used_inline_verbose = False
## Treating presence of '(?x)' as sign of in-line verbose mode indicator.
## To be honest, if it actually is there for another reason the worst that
## happens is we don't let them know about verbose option when we could have
str_els = get_str_els(block_el)
for str_el in str_els:
str_val = get_str_val(str_el)
if str_val and str_val.startswith(conf.INLINE_RE_VERBOSE_FLAG):
used_inline_verbose = True
break
return used_inline_verbose
def used_verbose(block_el):
if used_verbose_constant(block_el):
return True
if used_inline_verbose(block_el):
return True
return False
def used_compile(block_el):
pass
@snippet_advisor()
def verbose_option(blocks_dets):
"""
Check for use of regex without verbose mode and introduce the idea.
"""
has_imported_re = False
has_used_verbose = False
for block_dets in blocks_dets:
block_el = block_dets.element
if not has_imported_re:
if imported_re(block_el):
has_imported_re = True
if not has_used_verbose:
if used_verbose(block_el):
has_used_verbose = True
if has_imported_re and has_used_verbose:
break
needs_verbose = has_imported_re and not has_used_verbose
if not needs_verbose:
return None
title = layout("""\
### Option of using verbose mode with regex
""")
tm = '\N{TRADE MARK SIGN}'
brief_explain = layout(f"""\
Regular expressions are infamous for being hard to understand, hard to
debug, and hard to maintain / extend. It is sometimes said that when you
have a problem, and you solve it with regex, now you have two problems
;-). So anything you can do to make your regex more readable is a very
Good Thing{tm}. Which is where verbose mode is often helpful.
""")
longer_explain = (
layout("""\
Verbose mode lets you split your regex into smaller, more
manageable parts, and you can comment your intentions for each
part. The following example is hard enough to understand _with_
comments - imagine trying to make sense of it without! Or make
modifications without breaking everything. Good luck with that!
""")
+
layout('''\
pattern = r"""
(?P<before_id>
var\sdiv_icon_ ## var div_icon
\w+ ## lots of characters (don't care which until ...)
\s=\sL.divIcon\(\{[\s\S]*?\<div) ## = L.divIcon({ anything till <div (non-greedy)
(?P<after_id>[\s\S]*? ;) ## anything until first semi-colon (we're non-greedy
## so we can pick up multiple instances of pattern)
"""
''', is_code=True)
+
layout("""\
Note - because whitespace is ignored it must be explicitly
handled in your pattern. In some cases the overhead of doing
this will outweigh the benefits of using verbose mode.
""")
+
layout("""\
You can either use the verbose flag e.g.
""")
+
layout("""\
re.match(r'car', 'cardboard', flags=re.VERBOSE)
""", is_code=True)
+
layout("""\
or the in-line version of the flag e.g.
""")
+
layout("""\
re.match(r'(?x)car', 'cardboard')
""", is_code=True)
)
extra = layout("""\
The standard documentation at
<https://docs.python.org/3/howto/regex.html> is really good. Having said
that, regex is never trivial so don't worry if you find it challenging.
Pronunciation: reggex or rejex? Regex with a hard 'g' probably makes
most sense because regular also has a hard 'g' but there is no consensus
on pronunciation.
""")
message = {
conf.BRIEF: title + brief_explain,
conf.MAIN: title + longer_explain,
conf.EXTRA: extra,
}
return message
\ No newline at end of file
......@@ -13,7 +13,7 @@ RECORD_AST = f ## (f)
DEV_MODE = f ## (f)
DO_TEST = t ## set test snippet as default rather than the larger demo snippet (t)
DO_HTML = t ## set html displayer as default (t)
DO_DISPLAYER = f ## f is only ever used when testing pre-display (t)
DO_DISPLAYER = t ## f is only ever used when testing pre-display (t)
## =============================================================================
......@@ -27,12 +27,10 @@ else:
## When testing user-supplied snippets watch out for the BOM MS inserts via Notepad. AST chokes on it.
TEST_SNIPPET = """\
if apple or banana or cherry or date or elderberry:
pass
if date and elderberry and feijoia:
pass
if (apple or banana or cherry) and (date and elderberry and feijoia):
pass
#import re
from re import match
#flag = re.VERBOSE
#t_OPERATOR = r'(?x) <> | <= | >= | = | < | >'
"""
DEMO_SNIPPET = """\
......@@ -232,6 +230,9 @@ MISSING_ADVICE_MESSAGE = ("If there was some advice you think should have "
"'Advice' and explain. Please include a snippet to test as well.")
SYSTEM_MESSAGE = 'System message'
VERBOSE_FLAG = 'VERBOSE'
INLINE_RE_VERBOSE_FLAG = '(?x)'
LINE_FEED = '&#10;'
## scraped from https://docs.python.org/3/py-modindex.html 2020-04-02
......
......@@ -27,8 +27,20 @@ def get_message(message_dets, message_level):
message = md2cli.main(md=message.replace('`', '')) ## They create problems in formatting
return message
def _need_snippet_displayed(block_messages_dets, *, multi_block=False):
"""
Don't need to see the code snippet displayed when it is already visible:
* because there is only one block in snippet and there is a block message
for it (which will display the block i.e. the entire snippet)
Otherwise we need it displayed.
"""
mono_block_snippet = not multi_block
if mono_block_snippet and block_messages_dets:
return False
return True
def display(snippet, messages_dets, *,
message_level=conf.BRIEF, in_notebook=False):
message_level=conf.BRIEF, in_notebook=False, multi_block=False):
"""
Show by code blocks.
"""
......@@ -47,12 +59,15 @@ def display(snippet, messages_dets, *,
"""
)),
]
text.append(md2cli.main(dedent(
"## Overall Snippet"
f"\n{MDV_CODE_BOUNDARY}\n"
+ snippet
+ f"\n{MDV_CODE_BOUNDARY}")))
overall_messages_dets, block_messages_dets = messages_dets
display_snippet = _need_snippet_displayed(
block_messages_dets, multi_block=multi_block)
if display_snippet:
text.append(md2cli.main(dedent(
"## Overall Snippet"
f"\n{MDV_CODE_BOUNDARY}\n"
+ snippet
+ f"\n{MDV_CODE_BOUNDARY}")))
for message_dets in overall_messages_dets:
message = get_message(message_dets, message_level)
text.append(message)
......
......@@ -570,8 +570,24 @@ def repeat_overall_snippet(snippet):
html_strs.append(overall_code_str_highlighted)
return html_strs
def _need_snippet_displayed(block_messages_dets, *,
in_notebook=False, multi_block=False):
"""
Don't need to see the code snippet displayed when it is already visible:
* because in notebook (it is already in the cell straight above)
* because there is only one block in snippet and there is a block message
for it (which will display the block i.e. the entire snippet)
Otherwise we need it displayed.
"""
if in_notebook:
return False
mono_block_snippet = not multi_block
if mono_block_snippet and block_messages_dets:
return False
return True
def _get_all_html_strs(snippet, overall_messages_dets, block_messages_dets, *,
in_notebook=False):
in_notebook=False, multi_block=False):
"""
Display all message types - eventually will show brief and, if the user
clicks to expand, main instead with the option of expanding to show Extra.
......@@ -582,11 +598,9 @@ def _get_all_html_strs(snippet, overall_messages_dets, block_messages_dets, *,
all_html_strs = []
## overall snippet display
first_lines = {
message_dets.first_line_no for message_dets in block_messages_dets}
multi_block = (len(first_lines) > 1)
in_browser = not in_notebook
if in_browser and multi_block:
display_snippet = _need_snippet_displayed(block_messages_dets,
in_notebook=in_notebook, multi_block=multi_block)
if display_snippet:
overall_snippet_html_strs = repeat_overall_snippet(snippet)
all_html_strs.extend(overall_snippet_html_strs)
......@@ -634,7 +648,7 @@ def _get_head(*, in_notebook=False):
return head
def display(snippet, messages_dets, *,
message_level=conf.BRIEF, in_notebook=False):
message_level=conf.BRIEF, in_notebook=False, multi_block=False):
"""
Show for overall snippet and then by code blocks as appropriate.
......@@ -644,7 +658,8 @@ def display(snippet, messages_dets, *,
radio_buttons = _get_radio_buttons(message_level=message_level)
overall_messages_dets, block_messages_dets = messages_dets
all_html_strs = _get_all_html_strs(snippet,
overall_messages_dets, block_messages_dets, in_notebook=in_notebook)
overall_messages_dets, block_messages_dets,
in_notebook=in_notebook, multi_block=multi_block)
body_inner = '\n'.join(all_html_strs)
head = _get_head(in_notebook=in_notebook)
if in_notebook:
......
import argparse
import logging
from superhelp import conf
logging.basicConfig(
level=conf.LOG_LEVEL,
format='%(asctime)s %(levelname)-8s %(message)s',
......@@ -18,9 +20,10 @@ do_html = conf.DO_HTML ## use html displayer vs cli displayer
do_displayer = conf.DO_DISPLAYER ## for dev testing only
def display_messages(displayer, snippet, messages_dets, *,
message_level=conf.BRIEF, in_notebook=False):
res = displayer.display(snippet,
messages_dets, message_level=message_level, in_notebook=in_notebook)
message_level=conf.BRIEF, in_notebook=False, multi_block=False):
res = displayer.display(snippet, messages_dets,
message_level=message_level, in_notebook=in_notebook,
multi_block=multi_block)
if in_notebook:
return res
......@@ -65,17 +68,30 @@ def get_advice(snippet=None, *, file_path=None, displayer='html',
"""
snippet = _get_snippet(snippet, file_path)
try:
messages_dets = messages.get_separated_messages_dets(snippet)
messages_dets, multi_block = messages.get_snippet_dets(snippet)
except Exception as e:
messages_dets = messages.get_error_messages_dets(e, snippet)
multi_block = False
displayer_module = _get_displayer_module(displayer)
if displayer_module:
res = display_messages(displayer_module, snippet, messages_dets,
message_level=message_level, in_notebook=in_notebook)
message_level=message_level, in_notebook=in_notebook,
multi_block=multi_block)
if in_notebook:
return res
for comment in advisors.get_advisor_comments():
print(comment)
#TODO: allow extra in help
# for comment in advisors.get_advisor_comments():
# print(comment)
def shelp():
"""
......
......@@ -98,7 +98,7 @@ MessageDets.__doc__ += (
MessageDets.source.__doc__ = ("A unique identifier of the source of message "
"- useful for auditing / testing")
def get_tree(snippet):
def _get_tree(snippet):
try:
tree = ast.parse(snippet)
except SyntaxError as e:
......@@ -106,7 +106,7 @@ def get_tree(snippet):
f"Oops - something seems broken in the snippet - details: {e}")
return tree
def get_blocks_dets(xml, snippet_lines):
def get_blocks_dets(snippet, snippet_block_els):
"""
Returning a list of all the details needed to process a line
(namely BlockDets named tuples)
......@@ -116,18 +116,18 @@ def get_blocks_dets(xml, snippet_lines):
:return: list of BlockDets named tuples
:rtype: list
"""
snippet_lines = snippet.split('\n')
blocks_dets = []
all_elements = xml.xpath('body')[0].getchildren() ## [0] because there is only one body under root
for element in all_elements:
for snippet_block_el in snippet_block_els:
first_line_no, last_line_no, _el_lines_n = ast_funcs.get_el_lines_dets(
element)
snippet_block_el)
block_code_str = (
'\n'.join(snippet_lines[first_line_no - 1: last_line_no]).strip())
pre_block_code_str = (
'\n'.join(snippet_lines[0: first_line_no - 1]).strip()
+ '\n')
blocks_dets.append(BlockDets(
element, pre_block_code_str, block_code_str, first_line_no))
snippet_block_el, pre_block_code_str, block_code_str, first_line_no))
return blocks_dets
def complete_message(message, *, source):
......@@ -276,33 +276,21 @@ def get_overall_snippet_messages_dets(snippet, blocks_dets):
messages_dets.append(message_dets)
return messages_dets
def get_separated_messages_dets(snippet):
def get_separated_messages_dets(snippet, snippet_block_els, xml):
"""
Break snippet up into syntactical parts and blocks of code. Apply advisor
functions and get message details. Split into overall messages and block-
specific messages.
:param str snippet: code snippet
:param list snippet_block_els: list of block elements for snippet
:param xml xml: snippet code as xml object
:return: a tuple of two MessageDets lists
(overall_snippet_messages_dets, block_level_messages_dets)
or None if no messages
:rtype: tuple (or None)
"""
tree = get_tree(snippet)
xml = astpath.asts.convert_to_xml(tree)
if conf.RECORD_AST:
xml.getroottree().write(str(conf.AST_OUTPUT_XML), pretty_print=True)
logging.info("""\
Updating AST
""")
snippet_lines = snippet.split('\n')
blocks_dets = get_blocks_dets(xml, snippet_lines)
blocks_dets = get_blocks_dets(snippet, snippet_block_els)
overall_snippet_messages_dets = get_overall_snippet_messages_dets(
snippet, blocks_dets)
block_level_messages_dets = get_block_level_messages_dets(blocks_dets, xml)
......@@ -321,6 +309,21 @@ def get_separated_messages_dets(snippet):
first_line_no=None, warning=False, source=conf.SYSTEM_MESSAGE)]
return overall_snippet_messages_dets, block_level_messages_dets
def store_ast_output(xml):
xml.getroottree().write(str(conf.AST_OUTPUT_XML), pretty_print=True)
logging.info("\n\n\n\n\nUpdating AST\n\n\n\n\n")
def get_snippet_dets(snippet):
tree = _get_tree(snippet)
xml = astpath.asts.convert_to_xml(tree)
if conf.RECORD_AST:
store_ast_output(xml)
snippet_block_els = xml.xpath('body')[0].getchildren() ## [0] because there is only one body under root
multi_block_snippet = len(snippet_block_els) > 1
snippet_messages_dets = get_separated_messages_dets(
snippet, snippet_block_els, xml)
return snippet_messages_dets, multi_block_snippet
def get_error_messages_dets(e, snippet):
"""
If unable to produce any messages, supply the problem in the form of
......
## cd ~/projects/superhelp && superhelp/env/bin/python3 -m nose
import astpath
from nose.tools import assert_equal, assert_not_equal, assert_true, assert_false # @UnusedImport @UnresolvedImport
from superhelp import conf
from superhelp.messages import get_separated_messages_dets
from superhelp.messages import _get_tree, get_separated_messages_dets, \
store_ast_output
def get_actual_source_freqs(messages_dets, expected_source_freqs):
"""
......@@ -39,9 +40,13 @@ def check_as_expected(test_conf):
:param list test_conf: list of tuples: snippet, dict of expected message
sources and their expected frequencies
"""
conf.RECORD_AST = True ## updates XML so we can check what is happening :-)
for snippet, expected_source_freqs in test_conf:
messages_dets = get_separated_messages_dets(snippet)
tree = _get_tree(snippet)
xml = astpath.asts.convert_to_xml(tree)
store_ast_output(xml)
snippet_block_els = xml.xpath('body')[0].getchildren() ## [0] because there is only one body under root
messages_dets = get_separated_messages_dets(
snippet, snippet_block_els, xml)
actual_source_freqs = get_actual_source_freqs(
messages_dets, expected_source_freqs)
assert_equal(actual_source_freqs, expected_source_freqs,
......
from textwrap import dedent
from tests import check_as_expected
ROOT = 'superhelp.advisors.regex_advisors.'
def test_misc():
test_conf = [
(
dedent("""\
name_lengths = []
"""),
{
ROOT + 'verbose_option': 0,
}
),
(
dedent("""\
import re
"""),
{
ROOT + 'verbose_option': 1,
}
),
(
dedent("""\
import re
re.match(r'car', 'cardboard', flags=re.VERBOSE)
"""),
{
ROOT + 'verbose_option': 0,
}
),
(
dedent("""\
import re
re.match(r'(?x)car', 'cardboard')
"""),
{
ROOT + 'verbose_option': 0,
}
),
(
dedent("""\
import re
re.match(r'(?)car', 'cardboard')
"""),
{
ROOT + 'verbose_option': 1, ## (?x) isn't the (?x) in-line flag
}
),
(
dedent("""\
from re import match
match(r'(?x)car', 'cardboard')
"""),
{
ROOT + 'verbose_option': 0,
}
),
(
dedent("""\
from re import match
match(r'car', 'cardboard')
"""),
{
ROOT + 'verbose_option': 1,
}
),
(
dedent("""\
from re import match
match(r'car', 'cardboard', flags=re.VERBOSE)
"""),
{
ROOT + 'verbose_option': 0,
}
),
(
dedent("""\
from re import match, VERBOSE
match(r'car', 'cardboard', VERBOSE)
"""),
{
ROOT + 'verbose_option': 0,
}
),
]
check_as_expected(test_conf)
# test_misc()
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment