Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed broken functionalities when running on Python2 #263

Merged
merged 1 commit into from
Aug 10, 2019
Merged

Fixed broken functionalities when running on Python2 #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions mssqlcli/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,8 @@ def load_config(usr_cfg, def_cfg=None):

def ensure_dir_exists(path):
parent_dir = expanduser(dirname(path))
try:
if not os.path.exists(parent_dir):
os.makedirs(parent_dir)
except OSError as exc:
# ignore existing destination (py2 has no exist_ok arg to makedirs)
if exc.errno != errno.EEXIST:
raise


def write_default_config(source, destination, overwrite=False):
Expand Down
12 changes: 6 additions & 6 deletions mssqlcli/key_bindings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@ def mssqlcli_bindings(mssql_cli):
"""
key_bindings = KeyBindings()

@key_bindings.add('f2')
@key_bindings.add(u'f2')
def _(event):
"""
Enable/Disable SmartCompletion Mode.
"""
_logger.debug('Detected F2 key.')
mssql_cli.completer.smart_completion = not mssql_cli.completer.smart_completion

@key_bindings.add('f3')
@key_bindings.add(u'f3')
def _(event):
"""
Enable/Disable Multiline Mode.
"""
_logger.debug('Detected F3 key.')
mssql_cli.multiline = not mssql_cli.multiline

@key_bindings.add('f4')
@key_bindings.add(u'f4')
def _(event):
"""
Toggle between Vi and Emacs mode.
Expand All @@ -37,7 +37,7 @@ def _(event):
mssql_cli.vi_mode = not mssql_cli.vi_mode
event.app.editing_mode = EditingMode.VI if mssql_cli.vi_mode else EditingMode.EMACS

@key_bindings.add('tab')
@key_bindings.add(u'tab')
def _(event):
"""
Force autocompletion at cursor.
Expand All @@ -49,7 +49,7 @@ def _(event):
else:
b.start_completion(select_first=True)

@key_bindings.add('c-space')
@key_bindings.add(u'c-space')
def _(event):
"""
Initialize autocompletion at cursor.
Expand All @@ -67,7 +67,7 @@ def _(event):
else:
b.start_completion(select_first=False)

@key_bindings.add('enter', filter=has_selected_completion)
@key_bindings.add(u'enter', filter=has_selected_completion)
def _(event):
"""
Makes the enter key work as the tab key only when showing the menu.
Expand Down
5 changes: 4 additions & 1 deletion mssqlcli/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ def configure_and_update_options(options):
if not options.username:
options.username = input(u'Username (press enter for sa):') or u'sa'
if not options.password:
options.password = getpass.getpass()
pw = getpass.getpass()
pensivebrian marked this conversation as resolved.
Show resolved Hide resolved
if (pw is not None):
pw = pw.replace('\r', '').replace('\n', '')
options.password = pw


def create_config_dir_for_first_use():
Expand Down
6 changes: 3 additions & 3 deletions mssqlcli/mssql_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,11 +417,11 @@ def _build_cli(self, history):

def get_message():
prompt = self.get_prompt(self.prompt_format)
return [('class:prompt', prompt)]
return [(u'class:prompt', prompt)]

def get_continuation(width, line_number, is_soft_wrap):
continuation = self.multiline_continuation_char * (width - 1) + ' '
return [('class:continuation', continuation)]
return [(u'class:continuation', continuation)]

get_toolbar_tokens = create_toolbar_tokens_func(self)

Expand All @@ -446,7 +446,7 @@ def get_continuation(width, line_number, is_soft_wrap):
chars='[](){}'),
filter=HasFocus(DEFAULT_BUFFER) & ~IsDone()),
# Render \t as 4 spaces instead of "^I"
TabsProcessor(char1=' ', char2=' ')],
TabsProcessor(char1=u' ', char2=u' ')],
reserve_space_for_menu=self.min_num_menu_lines,

# Buffer options.
Expand Down
12 changes: 6 additions & 6 deletions mssqlcli/mssqlcompleter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,20 +142,20 @@ def extend_keywords(self, additional_keywords):
self.keywords.extend(additional_keywords)
self.all_completions.update(additional_keywords)

def extend_schemata(self, schemata):
def extend_schemas(self, schemas):

# schemata is a list of schema names
schemata = self.escaped_names(schemata)
# schemas is a list of schema names
schemas = self.escaped_names(schemas)
metadata = self.dbmetadata['tables']
for schema in schemata:
for schema in schemas:
metadata[schema] = {}

# dbmetadata.values() are the 'tables' and 'functions' dicts
for metadata in self.dbmetadata.values():
for schema in schemata:
for schema in schemas:
metadata[schema] = {}

self.all_completions.update(schemata)
self.all_completions.update(schemas)

def extend_casing(self, words):
""" extend casing data
Expand Down
43 changes: 32 additions & 11 deletions mssqlcli/mssqlqueries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,36 @@
import sys
import re

def get_schemas():
"""
Query string to retrieve schema names.
:return: string
"""
return '''
sql = '''
SELECT name
FROM sys.schemas
ORDER BY 1'''
return normalize(sql)


def get_databases():
"""
Query string to retrieve all database names.
:return: string
"""
return '''
sql = '''
Select name
FROM sys.databases
ORDER BY 1'''
return normalize(sql)


def get_table_columns():
"""
Query string to retrieve all table columns.
:return: string
"""
return '''
sql = '''
SELECT isc.table_schema,
isc.table_name,
isc.column_name,
Expand All @@ -49,14 +54,15 @@ def get_table_columns():
) AS ist
ON ist.table_name = isc.table_name AND ist.table_schema = isc.table_schema
ORDER BY 1, 2'''
return normalize(sql)


def get_view_columns():
"""
Query string to retrieve all view columns.
:return: string
"""
return '''
sql = '''
SELECT isc.table_schema,
isc.table_name,
isc.column_name,
Expand All @@ -80,72 +86,77 @@ def get_view_columns():
) AS ist
ON ist.table_name = isc.table_name AND ist.table_schema = isc.table_schema
ORDER BY 1, 2'''
return normalize(sql)


def get_views():
"""
Query string to retrieve all views.
:return: string
"""
return '''
sql = '''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't add a u string prefix and assume unicode?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can put u in front of ''' for making unicode. But we still need normalize() method for removing unintended spaces.

SELECT table_schema,
table_name
FROM INFORMATION_SCHEMA.VIEWS
ORDER BY 1, 2'''
return normalize(sql)


def get_tables():
"""
Query string to retrive all tables.
:return: string
"""
return '''
sql = '''
SELECT table_schema,
table_name
FROM INFORMATION_SCHEMA.TABLES
WHERE table_type = 'BASE TABLE'
ORDER BY 1, 2'''
return normalize(sql)


def get_user_defined_types():
"""
Query string to retrieve all user defined types.
:return: string
"""
return '''
sql = '''
SELECT schemas.name,
types.name
FROM
(
SELECT name,
schema_id
schema_id
FROM sys.types
WHERE is_user_defined = 1) AS types
INNER JOIN
(
SELECT name,
schema_id
schema_id
FROM sys.schemas) AS schemas
ON types.schema_id = schemas.schema_id'''
return normalize(sql)


def get_functions():
"""
Query string to retrieve stored procedures and functions.
:return: string
"""
return '''
sql = '''
SELECT specific_schema, specific_name
FROM INFORMATION_SCHEMA.ROUTINES
ORDER BY 1, 2'''
return normalize(sql)


def get_foreignkeys():
"""
Query string for returning foreign keys.
:return: string
"""
return '''
sql = '''
SELECT
kcu1.table_schema AS fk_table_schema,
kcu1.table_name AS fk_table_name,
Expand All @@ -164,3 +175,13 @@ def get_foreignkeys():
AND kcu2.constraint_name = rc.unique_constraint_name
AND kcu2.ordinal_position = kcu1.ordinal_position
ORDER BY 3, 4'''
return normalize(sql)


def normalize(sql):
if (sql == '' or sql is None):
return sql
else:
sql = sql.replace('\r', ' ').replace('\n', ' ').strip()
sql = re.sub(r' +', ' ', sql)
Copy link
Member

@pensivebrian pensivebrian Aug 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we didn't need these replace and substitutions before. Why now?

Copy link
Author

@geleems geleems Aug 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed some of our code recognized the type of command by looking at a word before the first space character. For example:

# mssqlcli\packages\special\main.py

def parse_special_command(sql):

    # split the sql by space -- 'command' is a word before first space.
    command, _, arg = sql.partition(' ')

    verbose = '+' in command
    command = command.strip().replace('+', '')
    return (command, verbose, arg.strip())

This assumes the word before the first space character is keyword of command (very fragile). Unfortunately we have methods that returns sql with bunch of leading white spaces. For example:

def get_sql():
    return '''
        SELECT col1,
               col2
        FROM t1
        '''

This method returns a string:

'\n        SELECT col1,\n               col2\n        FROM t1\n        '

The word before the first space is \n that is recognized as a command keyword. This not intended. And the leading \n prevents to strip all the white spaces in front of actual keyword. For example, sql.strip().startswith('SELECT') is false. And this way of peeking keyword is actually exists in our code.

It seems this behavior did not break existing functionality, and I guess it was because that type of parsing was only done for special command which starts with \\. (Neither \n or SELECT is special command so it has returned false anyway and did not break functionality -- working code relying on a bug.)

However, I still believe we should return a sql with correct format since there is an evidence authors assume the first word is a keyword for command.

The normalize() method I wrote converts the raw format into correct format like:

'SELECT col1, col2 FROM t1'

return sql.decode('utf-8') if sys.version_info[0] < 3 else sql
6 changes: 3 additions & 3 deletions tests/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@ def get_completer(self, settings=None, casing=None):
from mssqlcli.mssqlcompleter import MssqlCompleter
comp = MssqlCompleter(smart_completion=True, settings=settings)

schemata, tables, tbl_cols, views, view_cols = [], [], [], [], []
schemas, tables, tbl_cols, views, view_cols = [], [], [], [], []

for sch, tbls in metadata['tables'].items():
schemata.append(sch)
schemas.append(sch)

for tbl, cols in tbls.items():
tables.append((sch, tbl))
Expand Down Expand Up @@ -232,7 +232,7 @@ def get_completer(self, settings=None, casing=None):
ForeignKey(*fk) for fks in metadata['foreignkeys'].values()
for fk in fks]

comp.extend_schemata(schemata)
comp.extend_schemas(schemas)
comp.extend_relations(tables, kind='tables')
comp.extend_relations(views, kind='views')
comp.extend_columns(tbl_cols, kind='tables')
Expand Down