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

Run Pylint Against Node Trees Even If They Are Not External Files #3129

Closed
tbpassin opened this issue Feb 3, 2023 · 11 comments
Closed

Run Pylint Against Node Trees Even If They Are Not External Files #3129

tbpassin opened this issue Feb 3, 2023 · 11 comments
Assignees
Labels
Milestone

Comments

@tbpassin
Copy link
Contributor

tbpassin commented Feb 3, 2023

It would be useful to be able to run pylint against code trees even if they were not external files. An example use case would be for a complicated Leo command or script. It seems to me that the pylint command could write the subtree to a file and run pylint against that. It would be almost identical to launching it against an external file tree.

@tbpassin tbpassin added this to the 6.7.2 milestone Feb 3, 2023
@edreamleo
Copy link
Member

@tbpassin This idea is reasonable. Doing it cleanly and safly will likely take considerable work. I've delegated this issue to you :-)

@edreamleo edreamleo added the Delegated Delegatated to others label Feb 3, 2023
@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 3, 2023

Why am I not surprised?? Any tips on gottchas to look out for?

@edreamleo
Copy link
Member

@tbpassin

The problem might be straightforward. Leo already can run pyflakes checks on @button scripts. I'd start by finding that code and seeing what its constraints and assumptions are. Iirc, the code involves some special cases in Leo's execute-script logic.

But the problem is likely intractable.

Leo has beautify-files, blacken-files and pylint commands, but not beautify-tree, blacken-tree or not (as you propose) pylint-tree commands.

External programs such as black and pylint require syntactically correct python code. Sure, you could write part of an outline to a temp file, but what good would that do?

  • pylint: how to translate line numbers in the temp file to line numbers in the outline?
  • black: how to incorporate changes in the temp file back into the outline?

Summary

The problem is likely intractable because external programs such as black and pylint don't understand Leo's outline structure.

Leo's pylint-files and blacken-files commands do understand outline structure enough to create clickable links that work. My advice is to let well enough alone.

Edward

P.S. A long time ago I wrote horror-show code that related (iirc!!) to this general topic. For a while the horror code resided in the attic. I can't find it now. Perhaps deleting the code was a mistake. It might have served as a cautionary tale.

EKR

@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 4, 2023

It's the line numbers that I see as a potential stumbling block. The temp file would be (I think) exactly the same as what the subtree would be if it were made to be an external file, so I thought that the numbering code would work just as well. That remains to be tested, though.

@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 5, 2023

The easy part is done - getting the command to run against a subtree that isn't an external file. Now about those line number links ... we'll have to see. I have got an idea or two.

@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 5, 2023

This may not be too hard after all. I think the goal is to replace the file name in the strings returned by pylint (or another process managed by the bpm). If we are checking a subtree that is not an external file, then the returned strings will contain the temp file path instead of the external file path. We need to change them before the strings get to bpm.put_log().

This substitution needs to made in the bpm.on_idle() handler. If the handler can find the root node p, it can check to see if the subtree is an external file. If it isn't, and since it should also be able to find the temp file name, the idle handler can make the substitutions.

So we need to make sure that the idle handler can get root_node and filename. That shouldn't be too hard. At first glance, the modifications to existing code should be minor. If that's true, then the bpm will be able to run any of the bpm processes (not just pylint) against a subtree once they have been modified to go against a subtree that's not an external file.

This is starting to sound practical.

@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 8, 2023

A status report:

Pylint can be run against an arbitrary subtree of Python, and it displays the usual clickable links. It still works as it used to for external file trees. I've got one bug that somehow is preventing it from running on a non-external tree more than once. That should be fairly easy to set right.

This enhancement required a few coordinated changes to the Pylint class, to the BPM, and to the LeoLog.put_html_links. Once it's all working right, it should be easy to modify other bpm commands, like mypy and flake8, to also run against a non-external file tree if anyone thinks that would make sense (it may not).

@edreamleo
Copy link
Member

@tbpassin Well done, you!

@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 9, 2023

Log pane (the link is clickable and goes to the right line):

pylint: not an external file, using temp file
==== dispatching self.run_pylint() with data: [('/tmp/_ex_zvkf.py', <pos 140129508401280 childIndex: 5 lvl: 0 key: 140129509930288:5 run-ext-file>)] last_path: None
pylint: _ex_zvkf.py
************* Module _ex_zvkf
Node: run-ext-file:315:3: C1802: Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty (use-implicit-booleaness-not-len)](unl:///home/tom/.leo/workbook.leo#run-ext-file::-315)
pylint: finished

Also, the bug mentioned above is fixed. I will do some more testing before submitting a PR.

@tbpassin
Copy link
Contributor Author

tbpassin commented Feb 9, 2023

Status update:

  1. Found and fixed a previously-existing mypy bug that prevented it from actually running successfully.
  2. My approach for pylint to know if it is running against a non-external-file prevents it from running against all external files under an organizing node. This was a previous behavior. I can probably recover this behavior pretty easily.

@tbpassin
Copy link
Contributor Author

Implemented by 3140.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants