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

Jump to bottom

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

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

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3668b2b
plugins\create_package.py broad-exception-caught
evidencebp Nov 11, 2024
a15507c
plugins\new_resource_file\__init__.py superfluous-parens
evidencebp Nov 11, 2024
931b6fd
plugins\command_completions\__init__.py superfluous-parens
evidencebp Nov 12, 2024
82a16c6
plugins\file_conversion.py too-many-return-statements
evidencebp Nov 12, 2024
55eeec9
plugins\syntax_dev\completions.py too-many-return-statements
evidencebp Nov 12, 2024
6b23492
plugins\syntax_dev_legacy.py too-many-branches
evidencebp Nov 12, 2024
4d138d6
plugins\lib\fileconv\dumpers.py unnecessary-pass
evidencebp Nov 18, 2024
d91194c
plugins\lib\fileconv\loaders.py unnecessary-pass
evidencebp Nov 18, 2024
4bd5577
Revert "plugins\file_conversion.py too-many-return-statements"
evidencebp Nov 18, 2024
757e22c
Revert "plugins\syntax_dev_legacy.py too-many-branches"
evidencebp Nov 18, 2024
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
158 changes: 117 additions & 41 deletions plugins/file_conversion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
__all__ = ('PackagedevConvertCommand',)



# build command
class PackagedevConvertCommand(sublime_plugin.WindowCommand):
"""Convert a file (view's buffer) of type ``source_format`` to type
Expand Down Expand Up @@ -47,6 +48,104 @@ class PackagedevConvertCommand(sublime_plugin.WindowCommand):
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.")
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. "
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")
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)

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

return result



def run(self, source_format=None, target_format=None, ext=None,
open_new_file=False, rearrange_yaml_syntax_def=False, _output=None, **kwargs):
"""Available parameters:
Expand Down Expand Up @@ -96,47 +195,24 @@ def run(self, source_format=None, target_format=None, ext=None,
"""
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

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

# Load inline options
Loader = loaders.get[source_format]
opts = Loader.load_options(self.view)
Expand Down Expand Up @@ -191,14 +267,12 @@ def on_select(index):
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,
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,
source_format,
target_format)
if result:
return result

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

Expand All @@ -214,16 +288,18 @@ def on_select(index):
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