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.

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

Pylint alerts corrections as part of an intervention experiment 401 #402

Closed
wants to merge 10 commits into from
2 changes: 1 addition & 1 deletion plugins/command_completions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class SublimeTextCommandCompletionPythonListener(sublime_plugin.EventListener):
@inhibit_word_completions
def on_query_completions(self, view, prefix, locations):
loc = locations[0]
python_arg_scope = ("source.python meta.function-call.arguments.python string.quoted")
python_arg_scope = "source.python meta.function-call.arguments.python string.quoted"
if not view.score_selector(loc, python_arg_scope) or not is_plugin(view):
return None

Expand Down
4 changes: 3 additions & 1 deletion plugins/create_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ def _create_package(name):
os.mkdir(path)
except FileExistsError:
logger.error("Path exists already: %r", path)
except Exception:
except FileNotFoundError:
logger.error("Parent path does not exist: %r", path)
except OSError:
logger.exception("Unknown error while creating path %r", path)
else:
return path
Expand Down
158 changes: 117 additions & 41 deletions plugins/file_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
__all__ = ('PackagedevConvertCommand',)



# build command

Check failure on line 18 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

too many blank lines (3)
class PackagedevConvertCommand(sublime_plugin.WindowCommand):

Check failure on line 19 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

expected 2 blank lines, found 3
"""Convert a file (view's buffer) of type ``source_format`` to type
``target_format``.

Expand Down Expand Up @@ -47,7 +48,105 @@
kwargs={"target_format": "yaml", "default_flow_style": False})
)

def _auto_detect_file_type(self, source_format, target_format, output):
"""Available parameters:

source_format (str) = None
The source format. Any of "yaml", "plist" or "json".
If `None`, attempt to automatically detect the format by extension, used syntax
highlight or (with plist) the actual contents.

target_format (str) = None
The target format. Any of "yaml", "plist" or "json".
If `None`, attempt to find an option set in the file to parse.
If unable to find an option, ask the user directly with all available format options.
output (OutputPanel) = None
"""

type_handling = None

# Auto-detect the file type if it's not specified
if not source_format:
output.write("Input type not specified, auto-detecting...")
for Loader in loaders.get.values():
if Loader.file_is_valid(self.view):
source_format = Loader.ext
Copy link
Member

Choose a reason for hiding this comment

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

This source_format assignment is not returned.

output.print(' %s\n' % Loader.name)
break

if not source_format:
type_handling = output.print("\nUnable to detect file type.")

Check failure on line 78 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

multiple spaces after operator
elif target_format == source_format:
type_handling = output.print("File already is %s." % Loader.name)

return type_handling

def _validate_run(self, source_format=None, target_format=None):
"""Available parameters:

source_format (str) = None
The source format. Any of "yaml", "plist" or "json".
If `None`, attempt to automatically detect the format by extension, used syntax
highlight or (with plist) the actual contents.

target_format (str) = None
The target format. Any of "yaml", "plist" or "json".
If `None`, attempt to find an option set in the file to parse.
If unable to find an option, ask the user directly with all available format options.
"""

result = False

# Check the environment (view, args, ...)
if self.view.is_dirty():
# Save the file so that source and target file on the drive don't differ
self.view.run_command("save")
if self.view.is_dirty():
result = sublime.error_message("The file could not be saved correctly. "

Check failure on line 105 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

multiple spaces after operator
Copy link
Member

Choose a reason for hiding this comment

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

sublime.error_message returns None, so this breaks the logic in the calling function because it doesn't forward the function exit.

The various return some_function_call_that_returns_none are actually just shortcuts for some_function(); return because the return value of run is not inspected.

That's not clean code, obviously, but I wrote this over 10 years ago and didn't put code purity in as high of a regard as I do now. 😅

"The build was aborted")

Check failure on line 106 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

continuation line under-indented for visual indent
elif source_format and target_format == source_format:
result = True
self.status("Target and source file format are identical. (%s)" % target_format)

elif source_format and source_format not in loaders.get:
result = True
self.status("Loader for '%s' not supported/implemented." % source_format)

elif target_format and target_format not in dumpers.get:
result = True
self.status("Dumper for '%s' not supported/implemented." % target_format)

return result

def _revalidate_run(self, output, source_format=None, target_format=None,):
"""Available parameters:

source_format (str) = None
The source format. Any of "yaml", "plist" or "json".
If `None`, attempt to automatically detect the format by extension, used syntax
highlight or (with plist) the actual contents.

target_format (str) = None
The target format. Any of "yaml", "plist" or "json".
If `None`, attempt to find an option set in the file to parse.
If unable to find an option, ask the user directly with all available format options.
output (OutputPanel) = None
"""
result = None
# Validate the shit again, but this time print to output panel
if source_format is not None and target_format == source_format:
result = output.print("\nTarget and source file format are identical. (%s)"
Copy link
Member

Choose a reason for hiding this comment

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

output.print returns None as well.

% target_format)

Check failure on line 139 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

continuation line under-indented for visual indent

if target_format not in dumpers.get:
result = output.print("\nDumper for '%s' not supported/implemented."
% target_format)

Check failure on line 143 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

continuation line under-indented for visual indent

return result



def run(self, source_format=None, target_format=None, ext=None,

Check failure on line 149 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

too many blank lines (3)
open_new_file=False, rearrange_yaml_syntax_def=False, _output=None, **kwargs):
"""Available parameters:

Expand Down Expand Up @@ -96,47 +195,24 @@
"""
self.view = self.window.active_view()

# Check the environment (view, args, ...)
if self.view.is_dirty():
# Save the file so that source and target file on the drive don't differ
self.view.run_command("save")
if self.view.is_dirty():
return sublime.error_message("The file could not be saved correctly. "
"The build was aborted")

result = self._validate_run(self, source_format, target_format)
Copy link
Member

Choose a reason for hiding this comment

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

Remove self from the arguments.

By the way, you can also use the Python 3.8 walrus/assignment operator since this code targets Python 3.8 now.

if result:
return result

Check warning on line 201 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

blank line contains whitespace
file_path = self.view.file_name()
if not file_path:
return self.status("File does not exist.", file_path)
file_path = Path(file_path)

if source_format and target_format == source_format:
return self.status("Target and source file format are identical. (%s)" % target_format)

if source_format and source_format not in loaders.get:
return self.status("Loader for '%s' not supported/implemented." % source_format)

if target_format and target_format not in dumpers.get:
return self.status("Dumper for '%s' not supported/implemented." % target_format)

# Now the actual "building" starts (collecting remaining parameters)
with OutputPanel.create(self.window, "package_dev",
read_only=True, force_writes=True) as output:
output.show()

# Auto-detect the file type if it's not specified
if not source_format:
output.write("Input type not specified, auto-detecting...")
for Loader in loaders.get.values():
if Loader.file_is_valid(self.view):
source_format = Loader.ext
output.print(' %s\n' % Loader.name)
break

if not source_format:
return output.print("\nUnable to detect file type.")
elif target_format == source_format:
return output.print("File already is %s." % Loader.name)

type_handling = self._auto_detect_file_type(source_format, target_format, output)
if type_handling:
return type_handling

Check warning on line 215 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

blank line contains whitespace
# Load inline options
Loader = loaders.get[source_format]
opts = Loader.load_options(self.view)
Expand Down Expand Up @@ -191,14 +267,12 @@
return

target_format = opts['target_format']
# Validate the shit again, but this time print to output panel
if source_format is not None and target_format == source_format:
return output.print("\nTarget and source file format are identical. (%s)"
% target_format)

if target_format not in dumpers.get:
return output.print("\nDumper for '%s' not supported/implemented."
% target_format)
result = self._revalidate_run(self,

Check warning on line 270 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

trailing whitespace
Copy link
Member

Choose a reason for hiding this comment

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

Remove self from the arguments.

output,

Check warning on line 271 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

trailing whitespace
source_format,

Check warning on line 272 in plugins/file_conversion.py

View workflow job for this annotation

GitHub Actions / Flake8

trailing whitespace
target_format)
if result:
return result

output.print(' %s\n' % dumpers.get[target_format].name)

Expand All @@ -214,16 +288,18 @@
output.print("Unexpected error occurred while parsing, "
"please see the console for details.")
raise
if not data:
return

# Determine new file name
new_file_path = file_path.with_suffix(get_new_ext(target_format))
new_dir = new_file_path.parent
valid_path = True
try:
os.makedirs(str(new_dir), exist_ok=True)
except OSError:
output.print("Could not create folder '%s'" % new_dir)
valid_path = False

if not data or not valid_path:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think merging the two checks for data and for the path make sense here.

It does not increase readability since it's basically just replacing an immediate return with a variable assignment meaning "please return 3 lines below" in path case and it also changed the code because now os.makedirs is called even when data is falsy. Now, that probably won't matter in practice, but this isn't something that should happen when addressing linter warnings.

return

# Now dump to new file
Expand Down
2 changes: 1 addition & 1 deletion plugins/new_resource_file/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,4 @@ def _is_package_path(self, file_path):
for fp in (real_file_path, file_path):
if fp.startswith(pp):
leaf = fp[len(pp):].strip(os.sep)
return (os.sep not in leaf)
return os.sep not in leaf
31 changes: 18 additions & 13 deletions plugins/syntax_dev/completions.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,42 +232,47 @@
return all(self.view.match_selector(point + offset, selector)
for point in locations)

result = None

# None of our business
if not match_selector("- comment - (source.regexp - keyword.other.variable)"):
return None
result = None
Copy link
Member

Choose a reason for hiding this comment

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

Since result is already defined earlier, this could be a pass and can be interpreted as "don't change the default return value".


# Scope name completions based on our scope_data database
if match_selector("meta.expect-scope, meta.scope", -1):
return self._complete_scope(prefix, locations)
elif match_selector("meta.expect-scope, meta.scope", -1):
result = self._complete_scope(prefix, locations)

# Auto-completion for include values using the 'contexts' keys and for
if match_selector(
elif match_selector(
"meta.expect-context-list-or-content | meta.context-list-or-content",
-1,
):
return ((self._complete_keyword(prefix, locations) or [])
result = ((self._complete_keyword(prefix, locations) or [])
+ self._complete_context(prefix, locations))

Check failure on line 251 in plugins/syntax_dev/completions.py

View workflow job for this annotation

GitHub Actions / Flake8

continuation line under-indented for visual indent

# Auto-completion for include values using the 'contexts' keys
if match_selector(
elif match_selector(
"meta.expect-context-list | meta.expect-context | meta.include | meta.context-list",
-1,
):
return self._complete_context(prefix, locations) or None
result = self._complete_context(prefix, locations) or None

# Auto-completion for branch points with 'fail' key
if match_selector(
elif match_selector(
"meta.expect-branch-point-reference | meta.branch-point-reference",
-1,
):
return self._complete_branch_point()
result = self._complete_branch_point()

# Auto-completion for variables in match patterns using 'variables' keys
if match_selector("keyword.other.variable"):
return self._complete_variable()
elif match_selector("keyword.other.variable"):
result = self._complete_variable()

# Standard completions for unmatched regions
return self._complete_keyword(prefix, locations)
else:
# Standard completions for unmatched regions
result = self._complete_keyword(prefix, locations)

Check warning on line 274 in plugins/syntax_dev/completions.py

View workflow job for this annotation

GitHub Actions / Flake8

blank line contains whitespace
return result

def _line_prefix(self, point):
_, col = self.view.rowcol(point)
Expand Down
Loading