Revive External Tools plugin
AbandonedPublic

Authored by dhaumann on Jan 4 2019, 9:09 PM.

Details

Reviewers
cullmann
gregormi
Commits
R40:865d9d42a843: KateToolRunner: API documentation
R40:019942159560: Config widget: edit on double click
R40:b12f9197582f: Move load/save code to KateExternalTool
R40:38a99051b624: Track itemChanged signal to enabled "Apply" button
R40:1ef8478a4414: Introduce enum class SaveMode
R40:c9ee7cc10258: Config Widget: Fix loading tools twice
R40:4e7845733ace: Use EditingTransaction for grouping undo/redo events
R40:5889482366c4: Support [x] Reload current document after execution
R40:d8a684e8889d: Factor out KateMacroExpander
R40:80d65241cba3: Make sure new tool have unique action collection name
R40:e5d0e92de730: Add API documentation and remove m_view variable
R40:ac63f5e9e6ae: Use factored out KateMacroExpander for macro expansion
R40:a4e6ebfddbc9: Fix comment
R40:2aa7fd9b18af: Revive External Tools plugin
R40:8a4762ba1811: Revive External Tools plugin
R40:6e2fcaa70f83: Config dialog: set buddies and remove QDialogButtonBox connections, done in C++
R40:b986ac5cbacd: Test for /home in ls output
R40:5da27b5679c7: SaveMode::CurrentDocument: Only save if modified to avoid unnecessary recompiles
R40:4582418e13c4: Move KateExternalToolsPluginView to kateexternaltoolsview.h/cpp
R40:6d035cec5100: Run clang-format
R40:19ff0aa73ac8: Translate warnings for failed tool runs
R40:db51af5f51a4: Cleanups & port away from KIcon
R40:6f6287a28fc8: Implement OutputMode::InsertInNewDocument
R40:f403447a9565: Revive KTextEditor::Command interface by moving tool execution to…
R40:5b6a0e9a838b: Start port to new KTextEditor plugin architecture
R40:d153bc35e0be: Create KateToolRunner on the heap to not block Kate
R40:e4115f74cf9f: Add some FIXMEs
R40:8cd355751d1e: Fix margin of config page
R40:182d65b7edf6: Remove ExternalToolRunner, was already factored out
R40:aa5f7782095c: Remove code duplication for editing a tool
R40:22cf336ffa5f: Apply clang-reformat
R40:fa5b773f0765: ToolDialog: Fix tab order
R40:0b58ca5f6f4e: Support adding new categories
R40:42642ebad416: Change Add Tool to Add Tool... to indicate a dialog opens
R40:fa97ff7f1e29: Command line: help <tool> shows "Starts the external tool <tool>"
R40:5e1aebb60696: Add some tools: gitk, git-cola, git blame, Google Selected Text, Execute…
R40:f85c3817b1f9: Cleanup includes
R40:8d762a710509: Add some predefined tools
R40:b18a56fe2d99: Remove ToolItem and use QStandardItem everywhere, since Drag & Drop otherwise…
R40:2c82159c7dc5: Menu Action: Fix categorization
R40:a76c3c7bed8d: Delete messy extView() helper function, not needed anymore
R40:d2b6f3b28953: Add support for categories, config widget still missing
R40:db38e8ac4d58: Add some FIXMEs for later
R40:8e8fbd9c0606: Config Widget: Make "Add" a popup menu with "Add Tool" and "Add Category"
R40:a0ef47a5c79e: Pass KTextEditir::View to ToolRunner, since the view is needed later
R40:1ab932ef8470: Config Widget: Expand all, show category Uncategorized always first
R40:a9b612688dc7: Factor out KateToolRunner for better unit testing
R40:5431990a65e7: Config Widget: Allow drag & drop to reorder and recategorize
R40:68f4cbb33c4f: More porting to new KTextEditor plugin API
R40:d4b532c05079: Use qWarning() for exitCode!=0 and crashes (suboptimal)
R40:288271769656: Remove 'command' field in favor of 'input' for stdin
R40:759d264f1866: Load & save arguments
R40:31b18e798948: Use new-style connect, cleanup dialog setup
R40:e16cadc488d9: Compiles for the first time, lots of #ifdef 0
R40:e453ba560617: Preliminary test for loading & saving external tool data
R40:6f36d5a58f32: Fix modelines + clang-format
R40:2847d717f094: Refactor mimetype checking
R40:29ca5b9217a4: Config Widget: Remove up & down buttons, drag & drop does the same
R40:e551098deb51: Start implementing KateToolRunner
R40:2c5aabde96c5: Rename member variable config to m_config
R40:8328b32bd952: Adapt default tools to new config file format
R40:7baa56818a44: Port UI to Qt Designer file
R40:4cf6db0a1103: Config Widget: Start porting to QStandardItemModel
R40:412b19258b5a: More porting to new KTextEditor plugin API
R40:03c9a2af972c: KateExternalToolMenuAction: Use tools from plugin
R40:ca4bddc9ca8f: Return const ref
R40:3db6a18f5bc8: Also macro expand Executable and Input
R40:370693b0b695: Remove KateExternalToolAction in favor of simply using QAction with QAction…
R40:84c022d450a7: Move externaltools.h/cpp to KateExternalToolsView.h/cpp
R40:5d99d83bad45: External tool: load & save "Reload current document after execution"
R40:25aab8d41420: Make KateExternalTool::save() const
R40:9cfe5793f084: Plugin: save Tool pointers, since these stable pointers will be reused by the…
R40:64c729b6c019: KateToolRunner: Support passing stdin to process
R40:601d171f6dc4: Use translation domain kateexternaltoolsplugin
R40:aeca479e3126: KateToolRunner now takes ownership of the passed KateExternalTool
R40:f7eb83d748ab: Tools Menu: Show categorized actions first, then uncategorized
R40:0f620455283b: Rename 'tryexe' to 'executable'
R40:f1ac1b725f26: Add checkbox "[x] Include output from stderr and port to QRegularExpression
R40:ff05d15b83b8: Rename externaltools -> m_externalToolsMenu and make it private
R40:16c83351fd32: Simplify load & save of external tools
R40:51bb78fff9b3: External Tool: Add output modes
R40:111a1398d440: Initialize member variables
R40:03b43c3cc829: Start porting to KTextEditor plugin API
R40:8d9dd836ca15: Rename acname to actionName
R40:2c2fc3e9003d: Rename test to a more generic name
R40:431d7c21ef1f: Pass plugin pointer to actions, since the plugin will act as model
R40:00b7cfc7d9f3: Merge branch 'master' into revive-externaltools-plugin
R40:e2613c8f200a: Cleanup
R40:1c770b3975e3: Use signal&slot to trigger reload of a menu
R40:2bc1ac637d3e: Delete support for separators. Support for categories will be added later
R40:d8151a54605f: Adapt to json based plugin factory pattern
R40:d3dd0cbf8a86: Minor cleanups
R40:b88d65684a7e: Make KateExternalTool copyable, behavior is now value semantics
R40:652a6e2b66bb: Add support for working directory, if provided
R40:7026a5c2d498: Adapt unit test
R40:4646dbd5cb45: Simplify serialization and deserialization
R40:aa41b576aea0: KateToolRunner: Use std::unique_ptr to express ownership and avoid memleaks
R40:a038e3f5fa90: Factor out KateExternalTool into separate file
R40:be01ec2d9f4b: Add ExternalToolRunner unit test skel
R40:a5d74c09de0d: Add unit test that tests stdin input
R40:dde3169d7d23: Factor out KateExternalToolsConfigWidget into separate file
R40:66fbe76d2afe: Start implementing output handling
R40:e78763e9e1a0: Add operator== for KateExternalTool, used for unit test
R40:a95a835deae7: Rename kateexternaltoolsplugin to externaltoolsplugin
R40:f087d9325569: Factor out KateExternalToolsCommand into separate file
R40:df5c87104051: Revive edit dialog
R40:8b5ab3f1b112: Fix connection
R40:ba97690a8700: Remove Q_GADGET, not needed
R40:de16e7978367: Copyright
R40:06226e0cc9df: Cleanup includes
R40:4c426fff02cc: Config Dialog: Always provide valid tool, simplifies pointer handling
Summary

This brings back a heavily improved, almost rewritten version of the External Tools plugin that was removed with commit e443c58df03e9fb8f26b67e86852f708d097517a for KDE 4.8.

Revival is motivated by the fact that we seem to add more and more tools in context menus which not always makes sense (e.g. having lots of hard-coded git tools in the Projects plugin). It makes more sense to e.g. enable launching git-cola as external tool, which was also used for testing: git-cola -r %{CurrentDocument:Path}

The config widget looks as follows: Drag & Drop is supported, inline renaming of Categories

Tool configuration dialog opens on double click or edit:

The Output Modes are as follows:

Ideas

  • There are still no default external tools. What I have in mind is to add a button "Templates" that lists as many tools as possible that we can think of. So by default, we'll have no tool at all in the list, but adding one from the presets will be very straight forward.
  • Make %project available in the macro expander, if the Projects plugin is loaded.
  • Launch external tool in a new embedded terminal
Test Plan

Added small unit test for the KateToolRunner class to make sure running a child process works.

Diff Detail

Repository
R40 Kate
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
dhaumann updated this revision to Diff 49909.Jan 19 2019, 10:55 PM
  • Factor out KateExternalTool into separate file
  • Add ExternalToolRunner unit test skel
  • Factor out KateToolRunner for better unit testing
  • Introduce enum class SaveMode
  • Move load/save code to KateExternalTool
  • Rename member variable config to m_config
  • Initialize member variables
  • Simplify serialization and deserialization
  • Rename acname to actionName
  • Adapt default tools to new config file format
  • Rename test to a more generic name
  • Preliminary test for loading & saving external tool data
  • Start implementing KateToolRunner
  • Test for /home in ls output
  • Add some FIXMEs for later
  • Add checkbox "[x] Include output from stderr and port to QRegularExpression

Long not in a working state, so no review required.

dhaumann updated this revision to Diff 49964.Jan 20 2019, 9:13 PM
  • Add support for working directory, if provided
  • Load & save arguments
  • ToolDialog: Fix tab order
  • Factor out KateMacroExpander
  • Make KateExternalTool copyable, behavior is now value semantics
  • Use factored out KateMacroExpander for macro expansion
  • Remove 'command' field in favor of 'input' for stdin
  • KateToolRunner now takes ownership of the passed KateExternalTool
dhaumann updated this revision to Diff 49972.Jan 20 2019, 10:53 PM
  • KateToolRunner: Support passing stdin to process
  • Add unit test that tests stdin input
dhaumann updated this revision to Diff 50028.Jan 21 2019, 9:41 PM
  • Factor out KateExternalToolsCommand into separate file
  • Revive KTextEditor::Command interface by moving tool execution to KateExternalToolsPlugin
  • Pass plugin pointer to actions, since the plugin will act as model
  • Plugin: save Tool pointers, since these stable pointers will be reused by the actions
  • KateExternalToolMenuAction: Use tools from plugin
  • Remove KateExternalToolAction in favor of simply using QAction with QAction::data()
  • Minor cleanups
dhaumann updated this revision to Diff 50143.Jan 23 2019, 8:41 PM
  • Remove ExternalToolRunner, was already factored out
  • Create KateToolRunner on the heap to not block Kate
  • Run clang-format
  • Fix comment
  • Factor out KateExternalToolsConfigWidget into separate file
  • Rename externaltools -> m_externalToolsMenu and make it private
  • Use signal&slot to trigger reload of a menu
  • Delete messy extView() helper function, not needed anymore
  • Add API documentation and remove m_view variable
  • Cleanup includes
  • Delete support for separators. Support for categories will be added later
dhaumann updated this revision to Diff 50145.Jan 23 2019, 9:27 PM
  • Use translation domain kateexternaltoolsplugin
  • Add support for categories, config widget still missing
  • Refactor mimetype checking
dhaumann updated this revision to Diff 50559.Jan 30 2019, 3:15 PM
  • Remove Q_GADGET, not needed
  • Make KateExternalTool::save() const
  • Add operator== for KateExternalTool, used for unit test
  • Return const ref
  • Config Widget: Start porting to QStandardItemModel
  • Move externaltools.h/cpp to KateExternalToolsView.h/cpp
  • Move KateExternalToolsPluginView to kateexternaltoolsview.h/cpp
  • Cleanup includes
  • Config dialog: set buddies and remove QDialogButtonBox connections, done in C++
  • Menu Action: Fix categorization
  • Config Dialog: Always provide valid tool, simplifies pointer handling
  • Tools Menu: Show categorized actions first, then uncategorized
  • Simplify load & save of external tools
  • External Tool: Add output modes
  • External tool: load & save "Reload current document after execution"
  • Config Widget: Fix loading tools twice
  • Config Widget: Expand all, show category Uncategorized always first
dhaumann updated this revision to Diff 50608.Jan 31 2019, 4:38 PM

Config Dialog almost done:

  • Config widget: edit on double click
  • Config Widget: Allow drag & drop to reorder and recategorize
  • Config Widget: Remove up & down buttons, drag & drop does the same
  • Config Widget: Make "Add" a popup menu with "Add Tool" and "Add Category"
  • Support adding new categories
  • Make sure new tool have unique action collection name
  • Track itemChanged signal to enabled "Apply" button
  • Remove code duplication for editing a tool
dhaumann updated this revision to Diff 50615.Jan 31 2019, 7:19 PM

Work on tool runner:

  • Also macro expand Executable and Input
  • Pass KTextEditir::View to ToolRunner, since the view is needed later
  • Adapt unit test
  • KateToolRunner: API documentation
  • Start implementing output handling
dhaumann edited the summary of this revision. (Show Details)Jan 31 2019, 7:30 PM
dhaumann edited the test plan for this revision. (Show Details)
dhaumann updated this revision to Diff 50616.Jan 31 2019, 7:39 PM
dhaumann edited the summary of this revision. (Show Details)
  • Use EditingTransaction for grouping undo/redo events
  • Implement OutputMode::InsertInNewDocument
dhaumann edited the summary of this revision. (Show Details)Jan 31 2019, 7:40 PM

The plugin is progressing nicely. In fact, it's almost done and just a matter of polishing. I think it starts to be ready for testing.

Actions items still include:

  • "Reload current document after execution": Currently not yet implemented, since I think I will run into issues with the KDirWatcher of KTextEditor. This could be a tricky one.
  • "Display in Pane" is not yet implemented. My current plan is that each run creates a new ToolView on the bottom, using a QTextEdit with fixed font that displays the output. However, how to close this? The only solution I can think of right now is to add a "Close" button.
  • The variables %{directory} etc all work, but currently cannot nicely be inserted via GUI. The drop-down buttons in the tool dialog are supposed to do this, but maybe there is even a more elegant way.
  • I would like to change %{directory} to %{CurrentDocument:Path} etc, so match the variables of QtCreator.
dhaumann updated this revision to Diff 50620.Jan 31 2019, 9:14 PM
  • Use qWarning() for exitCode!=0 and crashes (suboptimal)
  • Translate warnings for failed tool runs
  • Command line: help <tool> shows "Starts the external tool <tool>"
  • Add some tools
    • gitk
    • git-cola
    • git blame
    • Google Selected Text
    • Execute current file in Shell

These tools are not visible in the UI yet, since the Presets button
is not yet implemented.

dhaumann updated this revision to Diff 50673.Feb 1 2019, 9:19 PM
  • Add some predefined tools (currently only in the file externaltool, not yet in the UI)
  • Change Add Tool to Add Tool... to indicate a dialog opens
  • SaveMode::CurrentDocument: Only save if modified to avoid unnecessary recompiles
  • KateToolRunner: Use std::unique_ptr to express ownership and avoid memleaks
  • Support [x] Reload current document after execution
  • Remove ToolItem and use QStandardItem everywhere, since Drag & Drop otherwise messes up the type

Btw, for the presets in the file 'externaltools' I have the following suggestion: Add "Presets >" with submenus to the Add button. The Add button would the contain:

  • Add Tool...
  • Add Preset > ...
  • <separator>
  • Add Category

Would that be a good idea? Or would you prefer another button, i.e.: Add | Edit... | Remove <spacer> Presets // here Presets would be a drop down menu listing all the presets. Or better call it Templates?

Template sounds better than preset ;=)

I play shortly with the thing.

Really important would be the implementation that stderr ends up in a extra pane.

For the pane: I would open one toolview for the external plugin output and have just consecutive runs overwrite the output.
One might want to implement later some kind of "history", e.g. remember the last X execution outputs.
But I wouldn't do one output pane per run.

I played a bit with writing some perl script, but actually nothing happend, even if I include stderr to replace my document.
I assume my command failed.

That would be something for the external tools toolview, too.

Having there tabs, aka one for the output you requested and a second to have some meta info of what was executed. e.g. full command line printed with working dir and stuff.

gregormi added a comment.EditedFeb 3 2019, 5:40 PM

I tried the plugin, these are my observations:

  • From the "Edit Tool" GUI I found no help which variables can be used. Apart from the dropdown menu, a simple tooltip would serve the purpose as a first step.
    • "I would like to change %{directory} to %{CurrentDocument:Path} etc, so match the variables of QtCreator." +1
  • For the following test I used this command: bash with arguments: -c "echo %{filename}"
    • Output "Display in Pane": has no effect
      • ""Display in Pane" is not yet implemented. My current plan is that each run creates a new ToolView on the bottom, using a QTextEdit with fixed font that displays the output. However, how to close this? The only solution I can think of right now is to add a "Close" button." --> I am with Christoph: only one pane, later with history. Idea: Why an extra Close button? If it should behave like regular toolview, it can just stay open. Or by configuration only be shown on error. Instead of "Close" the button could be called "Go back" or so to focus the last active tool view.
    • Output "Insert in New Document": works fine
    • Output "Append to Current Document": works fine
    • Output "Replace Current Document": works fine. Idea: restore cursor position after operation.
    • Output "Replace Selected Text": works fine
    • Output "Insert at Cursor Position": works fine, even if text is selected
  • The file externaltools contains a number of tools. As I understand those should be made available under a "Add Preset" or "Add Template" button but are not visible by default which requires an initial user action, right? Thoughts about this:
    • Make these tools available by default in their respective category.
    • In the settings page those tools could appear with a postfix, e.g. "Run Shell Script [built-in]" or "Clang Format Full File [built-in] to make clear those are built-in
    • If the user needs to change one: Let him copy the built-in item and have the possibility to disable the original one. This way it will still be visible in the Tools manager for reference or later use.
    • The "built-in" postfix could also serve as a hint that this tool is proven not to do any damage because it is reviewed by the community.
  • It would be nice to have a way to copy an existing tool (built-in or custom) to be used as template for a new one. Can a tool be exported to make it shareable (ideally in the ini format of the externaltools file so that good tested tools can also be easily made available there)?
  • I tried to add this tool, working example: git gui blame -- README.md. Edit Tool: Executable: git, Arguments: gui blame -- %{filename}. The gui gui starts but not with the selected file to blame. When I set Working Directory to %{directory}, then it works which means the default setting which is hinted by the gray text "Use current document path if empty" does not work properly.
  • %{directory} expands to e.g. ///home/gregor/kde/src/kde/applications/kate/ Are the three slashes intended?
  • I set Executable to cowsay and Input: to %{selection}. As soon as there is a space in the selected text the input is automatically quoted with single-quotes which is visible in the output. Maybe instead of auto-quoting add a hint that some programs might need (manual) quoting to work properly.
  • "Edit Tool" minor GUI thing:
    • I would move "Mime types:" to the bottom of the dialog (or below "Name:") because it only affects the place or occasion when the tool is shown and has no operational effect.
    • Add a hint label that each tool is automatically available in "Configure Shortcuts" to set a custom shortcut.
    • "Editor command:": Side-question: is there a list of available commands in the Command Bar? E.g. pgrep is useful but hard to discover.
  • Ideas for later:
    • for Git: have a possibility to select a branch, tag or another revision as input, e.g. to make a "Diff of the current file (or folder) with another revision" tool.
    • for the Diff tool in the tab bar: have a variable that holds the path to the document under the inactive tab which was right-clicked.

The following list is my current proposal of variables. These variables already vary from Qt Creator. Not everything else matches anyways:

CurrentDocument:FileBaseName        - Current document: File base name without path and suffix.
CurrentDocument:FileName            - Current document: File name without path.
CurrentDocument:FilePath            - Current document: Full path including file name.
CurrentDocument:FileExtension       - Current document: File extension.
CurrentDocument:Text                - Current document: Contents of entire file.
CurrentDocument:Path                - Current document: Full path excluding file name.
CurrentDocument:NativeFilePath      - Current document: Full path including file name, with native path separator (backslash on Windows).
CurrentDocument:NativePath          - Current document: Full path excluding file name, with native path separator (backslash on Windows).

CurrentDocument:Cursor:Line         - Line number of the text cursor position in current document (starts with 0).
CurrentDocument:Cursor:Column       - Column number of the text cursor position in current document (starts with 0).
CurrentDocument:Cursor:XPos         - X component in global screen coordinates of the cursor position.
CurrentDocument:Cursor:YPos         - Y component in global screen coordinates of the cursor position.
CurrentDocument:Selection:Text      - Current document: Full path excluding file name.
CurrentDocument:Selection:StartLine
CurrentDocument:Selection:StartColumn
CurrentDocument:Selection:EndLine
CurrentDocument:Selection:EndColumn
CurrentDocument:RowCount            - Total number of lines in the current document.

Date:<format>                       - The current date (QDate formatstring).
Date:Locale                         - The current date in current locale format.
Date:ISO                            - The current date (ISO).

Time:<format>                       - The current time (QTime formatstring).
Time:Loca                           - The current time in current locale format.
Time:ISO                            - The current time (ISO).

Env:<value>                         - Access environment variables.

JS:<expression>                     - Evaluate simple JavaScript statements. The statements may not contain '{' nor '}' characters.

UUID                                - Generate a new UUID.

Any objections or additons to this?

@gregormi

  • Output "Replace Current Document": works fine. Idea: restore cursor position after operation.
    • Given the document usually changed afterwards, "restoring the cursor position" is pretty much undefined, and almost never correct...
  • Display in Pane: Working on it
  • Make tools available by default: I don't want to clutter the user with hundreds of tools. That's why I proposed the "Template". In general, I am not a big fan of adding everything "just because we can".
  • Copy existing tool: Noted, makes sense
  • Working directory if empty: fixed
  • The leading triple slsahes are not wanted, need to do more QUrl voodoo
  • Auto quoting: Currently I use KShell::splitArgs(m_tool->arguments) to get a list that I can pass to the QProcess. I assumed that this is correct in mooooost of the cases. Do we really want to have manual work here? Where is your bug? ;)
  • ideas for later: git, select a branch: I don't have plans to extend a general purpose tool (external tools) into something that needs a lot of special handling. If you have a general purpose solution, fine, otherwise: wish rejected.
dhaumann updated this revision to Diff 50859.Feb 4 2019, 4:07 PM
  • Fix margin of config page
  • Merge branch 'master' into revive-externaltools-plugin
  • Save & load enums manually instead of casting to int. This is more flexible once we change them
  • Register PluginView in Plugin, so that the PluginView for a MainWindow can be found
  • Start working on the tool view
  • Escape hides & deletes the tool view
  • Use toolview to report a crash
  • Working dir: if empty, use the current document's path
  • Pass also exitCode and crashed-flag in signal toolFinished()
  • API to clear & delete toolview
  • Expand empty working directory to current document URL only when URL is valid
  • Show output in toolview for "Display in Pane"
  • Better toolview handling (not perfect, yet)
  • Use fixed font for displaying output and status text
  • Implement new macro expansion (similar to Qt Creator)
  • Remove support for old macros
dhaumann updated this revision to Diff 50878.Feb 4 2019, 7:24 PM
  • Remove checkbox "[ ] Include output from stderr" in favour of always displaying it in the Status pane, if existing
  • Use QTextDocument as model to decouple ui from data
  • Macro expansion: fix global XPos and YPos
  • Write output and stderr to QTextDocuments directly
  • Add 'external-tool-test', testing all macros
dhaumann edited the summary of this revision. (Show Details)Feb 4 2019, 7:28 PM

@gregormi

  • Output "Replace Current Document": works fine. Idea: restore cursor position after operation.
    • Given the document usually changed afterwards, "restoring the cursor position" is pretty much undefined, and almost never correct...

Hmm. My idea came from this siutation: 1) have an XML document open, 2) cursor somewhere near the top, 3) Run Format document. => View is scrolled down to the bottom. More comfortable would feel if the view would stay somewhere around the original position (must not be exact). Especially when formatting almost correctly formatted documents the document does not change that much. In any case: a nice-to-have feature.

  • Make tools available by default: I don't want to clutter the user with hundreds of tools. That's why I proposed the "Template". In general, I am not a big fan of adding everything "just because we can".

I'm looking forward to having 100 tools :-). One thing came to my mind: what if we had externalstools.default and externaltools.extra? The tools defined in the first file are added by default. The second file contains those tools that must be added via "Templates". Personal note on the name "template": for a ready-made tool that can just be used as it is (which hopefully most of the tools are), the term "template" does not fit so good.

  • Auto quoting: Currently I use KShell::splitArgs(m_tool->arguments) to get a list that I can pass to the QProcess. I assumed that this is correct in mooooost of the cases. Do we really want to have manual work here? Where is your bug? ;)

My bug: cowsay Hello World

shows

 _______________ 
< 'Hello World' >
 --------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

via the plugin, instead of

 _____________ 
< Hello World >
 ------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

...let us call it a minor bug ;-) and set it aside for now.

  • ideas for later: git, select a branch: I don't have plans to extend a general purpose tool (external tools) into something that needs a lot of special handling. If you have a general purpose solution, fine, otherwise: wish rejected.

I thought about a general purpose solution: What about adding a new variable "ExternalTools:ScriptsPath" which points to a (new) directory "scripts" which is located in parallel to the externaltools file? In this directory scripts can be added that fulfill more special yet useful tasks, like the mentioned git tasks. I can also imagine more complex scripts that extract useful information out of the git repo and show it as HTML file (e.g. author statistics, extended blame or log info etc.) or help to extract a subfolder as a new git repository preserving its history.

By the way: idea for later for another Output type: "Show as popup" or "Display as inline note". This could be used for tools with little output like "Git Blame / Who touched this line last?" which runs only on the currently selected line.

The following list is my current proposal of variables. These variables already vary from Qt Creator. Not everything else matches anyways:
...
Any objections or additons to this?

Looks good. Is there a specific reason why the "variables already vary from Qt Creator"? I thought a goal was to make them as similar as possible. Otherwise fine.
I am curious: Is there a use case for the global cursor coordinates (in pixels, I suppose)?
For clang-format something like "CurrentDocument:Selection:StartBytes", "CurrentDocument:Selection:EndBytes", and "CurrentDocument:Selection:ByteCount" could be helpful.

CurrentDocument:Selection:Text - Current document: Full path excluding file name.
Time:Loca - The current time in current locale format.

minor copy&paste errors

  • cursor position: Ok I agree, indeed I was stumbling over this as well. So blindly set it to the previous position, and if possible keep the scroll position.
  • default vs not default: The problem with default tools is always that we have no idea what users will want to do. Adding e.g. git-cola (which imho makes sense) likely does not help 95% of the users. Especially others like qmlscene preview are even more special. I am not yet convinced to have all these as defaults in the list. I would rather want to add a "External Tools > Configure" entry to make configuration more accessible.
  • cowsay: I see. Still, I think in most cases you need escaping. I can have a look again into what KShell::splitArgs() supports...
  • KateExternalTools::ScriptsPath: Hm... that sounds like a nice try of adding something, along the lines of "QuickCoding" that never worked for users. As soon as it gets more complicated, noone except me (who write this) will understand anymore. ...or you need much more UI, which yet has to be invented.
  • I like the idea of "Show as popup" or "Display as inline note". I was misunderstanding this first, though: For "Display as inline note" I first thought of the new InlineNoteInterface, but I think you are talking about KTextEditor::Message.
  • Variables/Qt Creator: Because I think some are confusing. E.g. CurrentDocument:Column is less clear to me than CurrentDocument:Cursor:Column. Further, Qt Creator uses CurrentDocument::Row, where I prefer "Line", so it's CurrentDocument:Cursor:Line. For CurrentDate, I changed to Date, since it's shorter and another Date than "Current" does not make sense at all anyways (you would hard-code it then). Well... in the end, it probably does not matter much.
  • XPos, YPos: Yes, you could start a tool at a specific position of the screen, making it look as if you have a context menu on the cursor position. E.g. kdialog --geometry... (seems to not work, though). But I think it could be useful (Qt Creator has it, too)
dhaumann updated this revision to Diff 50912.Feb 5 2019, 8:49 AM
  • Merge branch 'master' into revive-externaltools-plugin
  • Improve messaging and toolview handling
  • Cleanup: Remove tool parameter from addToolStatus()
pino removed a subscriber: pino.Feb 5 2019, 8:59 PM
  • I like the idea of "Show as popup" or "Display as inline note". I was misunderstanding this first, though: For "Display as inline note" I first thought of the new InlineNoteInterface, but I think you are talking about KTextEditor::Message.

Actually, I meant both, KTextEditor::Message for "Show as popup" and InlineNoteInterface for the other one. For first one, there are some use cases; the second one could be used to display git blame information more persistently than with a disappearing note. Both nice to have, of course.

About the variables:
there might be the wish to do some action (formatting, annotating) on the current view, therefore the suggestion:

CurrentDocument:View:StartLine    - the first line that is at least partly visible in the current view
CurrentDocument:View:EndLine      - the last line that is at least partly visible in the current view

Maybe also because we have RowCount, but I have no use case yet:

CurrentDocument:CharCount            - Total number of characters in the current document
CurrentDocument:ByteCount            - Total number of bytes in the current document
dhaumann updated this revision to Diff 53482.Mar 8 2019, 9:41 PM
  • Cleanup What's This tip in Tool Dialog
  • Add a ContextAction that attaches to QLineEdits. Unfortunately QTextEdit (Input) is not a QLineEdit, so we need additional hacks to make ContextAction also attachable to QTextEdit (with additional child widget that is added / removed on focus in / out)
dhaumann updated this revision to Diff 53923.Mar 14 2019, 9:07 PM
  • Remove hard-coded variables in favor of registering variables

Here the link to the mentioned asyle stuff
https://community.kde.org/Policies/Kdelibs_Coding_Style#Artistic_Style_.28astyle.29_automatic_code_formatting
But there is still "--align-pointer=name" missing

addons/externaltools/externaltoolsplugin.cpp
233 ↗(On Diff #53923)

runner->view() is your view

262 ↗(On Diff #53923)

Looks to me that in case of auto view == nullptr you could return on top of function and avoid all further tests

addons/externaltools/externaltoolsplugin.h
53 ↗(On Diff #53923)

instantiate ?

70 ↗(On Diff #53923)

\@p cmd ?

addons/externaltools/kateexternaltool.h
65 ↗(On Diff #53923)

T

80 ↗(On Diff #53923)

command line ?

86 ↗(On Diff #53923)

you had always dots at the end, same one line above

90 ↗(On Diff #53923)

Is is set when load the tool from disk. ?

addons/externaltools/kateexternaltoolsconfigwidget.cpp
76 ↗(On Diff #53923)

Existing code use //BEGIN (no space)

257 ↗(On Diff #53923)

{ }

330 ↗(On Diff #53923)

search

addons/externaltools/kateexternaltoolsconfigwidget.h
41 ↗(On Diff #53923)

rm The config widget, starting with Allows

42 ↗(On Diff #53923)

and to

116 ↗(On Diff #53923)

S

138 ↗(On Diff #53923)

Coding style says QIcon &icon, always to the right. You reformat by some clang command? There was a astyle command somewhere offered. There are much more other places affected.

addons/externaltools/kateexternaltoolsview.h
91 ↗(On Diff #53923)

dot, also next comment

123 ↗(On Diff #53923)

; -> .

some cases above toolview -> tool view

addons/externaltools/katetoolrunner.h
74 ↗(On Diff #53923)

Blockingcall ... finished. or done?

dhaumann updated this revision to Diff 54071.Mar 17 2019, 9:58 AM
  • Use variables from KTextEditor Framework 5.57
dhaumann updated this revision to Diff 54267.Mar 18 2019, 9:09 PM
  • Require KTextEditor 5.57 and adapt to KTextEditor interface changes
This revision was not accepted when it landed; it landed in state Needs Review.Jun 10 2019, 2:02 PM
This revision was automatically updated to reflect the committed changes.
dhaumann reopened this revision.Jun 10 2019, 2:16 PM

It's not closed, it just lives in the kate/revive-externaltools-plugin branch.

dhaumann abandoned this revision.Thu, Sep 12, 8:48 AM

Merged!