Add Open in Editor support for template paths#2198
Add Open in Editor support for template paths#2198federicobond wants to merge 8 commits intodjango-commons:mainfrom
Conversation
5102676 to
543c143
Compare
tim-schilling
left a comment
There was a problem hiding this comment.
I wonder if it's possible to detect that the project is running in vscode and we could avoid the setting altogether?
|
Ah, that's a good reference. I like the idea of having a magic-ish value/constant to specify rather than asking them to write in a lambda for a setting. |
99f0190 to
418e706
Compare
|
@tim-schilling updated to use a constant to specify the editor URL format. Test failures appear unrelated. |
f844db5 to
bda95cf
Compare
tim-schilling
left a comment
There was a problem hiding this comment.
This is looking good. Marking it as Request Changes to ask for the tests. I think tests that cover get_editor_url and the new get_stats() of TemplatesPanel would be great.
The other stretch thing would be to add is a reference to the new setting in the TemplatesPanel documentation.
|
I just added the requested tests and updated the docs format. I would prefer to hold off with documenting this as something that's mostly useful for the requests panel because I believe it's going to be useful in other places too (like stacktraces). |
|
That's fair about mentioning the editor setting the templates panel documentation. I think we do want to have this mentioned in the changes.rst file so folks get alerted to the change in the next release. |
|
Done! |
5d4d274 to
943037d
Compare
tim-schilling
left a comment
There was a problem hiding this comment.
I'm good with these changes. I'd like @matthiask to agree (I don't want to merge a new feature unilaterally). He's out on holiday currently, so it may be a few weeks.
|
Thank you @federicobond for your patience and perseverance! |
ddd6bfd to
6bcd536
Compare
6bcd536 to
86c146c
Compare
|
Rebased against latest |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
@federicobond may I be so bold as to propose a slimmer solution?
Instead of hardcoding all the different URIs for all the different tool, why not just give the option to provide a URI as a setting?
If not, this should at least be the fallback. So if the URI is not in the dictionary, whatever string was provided is interpreted as a custom URI template.
| if template is None: | ||
| return None | ||
| return template.format(file=file, line=line) |
There was a problem hiding this comment.
| if template is None: | |
| return None | |
| return template.format(file=file, line=line) | |
| if template: | |
| return template.format(file=file, line=line) |
|
|
||
|
|
||
| def get_editor_url(file: str, line: int = 1) -> str | None: | ||
| formats = { |
There was a problem hiding this comment.
This would be a great application for the all new template strings.
| "debug_toolbar.panels.profiling.ProfilingPanel", | ||
| "debug_toolbar.panels.redirects.RedirectsPanel", | ||
| }, | ||
| "EDITOR": "vscode", |
There was a problem hiding this comment.
I don't think there should be a default. I'd rather have the feature disabled by default.
|
@codingjoe I think we're going to have a difference of opinion here. #2198 (comment) |
Yes, I saw that, which is why I proposed both at the end. Joining ease of use with flexibility for the obscure. It also doesn't require a lambda; the setting can remain a string. |
Description
This adds a setting to configure an Open in editor URL when you click on a template path. Makes it easier to jump directly to the template and edit. Can develop it further and/or add tests if there is interest in the feature.
Checklist:
docs/changes.rst.