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

Suggestion: Extend configuration to open current file with external editor #947

Closed
jbridgy opened this issue May 1, 2022 · 18 comments
Closed

Comments

@jbridgy
Copy link

jbridgy commented May 1, 2022

Currently the context menu of the mm editor contains the command Open With... which pops up the same dialog as the equally named command of the Windows File Explorer (explorer.exe). This is far from optimal if you want to edit the current file with another editor because of the following shortcomings:

  1. If the current file is modified then you are not asked whether you want to save it before switching to the other editor.
    Forgetting to do so can cause a lot of trouble.

  2. You have to select and run the other app in the mentioned dialog.
    The selection is annoying work if mm is the default app for *.md files which is the most likely case for mm users.

  3. The current position (line and column) is not passed to the other app.

It would be time-saving if the user could configure one or more external editors that can be invoked by clicking on a corresponding command in the context menu (eg: Edit With Notepad++) or even more convenient by pressing a shortcut key (eg: Ctrl+E).
Before invoking the external editor mm should offer a Save Document dialog if the current file is modified, that is:

<CurrentFile> has been modified.
Do you want to save changes?
[Yes] [No] [Cancel]

The settings for an external editor in MarkdownMonster.json could be:

Key Explanation
Label Text shown in context menu of mm editor.
CommandLine Path of external editor and parameters to be passed.
Shortcut To be warned on startup and ignored if shortcut is already used. Default: ""

EXAMPLE:

"ExternalCommands": [
  {
    "Label": "Edit With Notepad++",
    "CommandLine": "\"c:\\Program Files\\Notepad++\\notepad++.exe\" -n%l -c%c  \"%f\"",
    "Shortcut": "Ctrl+E"
  },
  {
    "Label": "Edit With Codium",
    "CommandLine": "\"c:\\Program Files\\VSCodium\\VSCodium.exe\" -g \"%f:%l:%c\""
  }
]

mm would expand the placeholders related to the position of the cursor in the mm editor as follows:

Placeholder Meaning
%f absolute path of current file
%l line number
%c column number

The option Shortcut may be too expensive for now because it implies that mm needs a shortcut manager. This is actually a proposal of its own.

@RickStrahl
Copy link
Owner

I'm not adding this. Open With... is a reasonable way to get to other documents. If you need more control, you can create a Command Script or an Addin.

Here's a commander script that lets you do what you want:

var docFile = Model.ActiveDocument.Filename;
if (string.IsNullOrEmpty(docFile))
	return null;

var exe = @"C:\Program Files\Microsoft VS Code\Code.exe";
exe = Environment.ExpandEnvironmentVariables(exe);

Process pi = null;
try {
	var lineNo = await Model.ActiveEditor.GetLineNumber();
	pi = Process.Start(exe,"-g \"" + docFile + $":{lineNo}\"");
}
catch(Exception ex) {
	Model.Window.ShowStatusError("Couldn't open editor: " + ex.Message);
	return null;
}

if (pi != null)
    Model.Window.ShowStatus($"VS Code  started with: {docFile}",5000);
else
    Model.Window.ShowStatusError("Failed to start VS Code.");

image

@RickStrahl
Copy link
Owner

Some of this functionality will be added in 2.7.8.

Run External Programs via Open With...

@jbridgy
Copy link
Author

jbridgy commented Jan 5, 2023

I tested mm 2.7.10.2 with the following configuration:

  "ExternalPrograms": [
    {
      "Name": "Command 1: Open With Notepad using {CurrentFile}",
      "Executable": "C:\\Windows\\System32\\notepad.exe",
      "Args": "\"{CurrentFile}\"",
      "Extensions": ""
    },
    {
      "Name": "Command 2: Open With Notepad using {ToClipBoardText}",
      "Executable": "C:\\Windows\\System32\\notepad.exe",
      "Args": "{ToClipBoardText}",
      "Extensions": ""
    }
  ],

Command 1 succeeded.
Command 2 always failed with the following error message regardless of the content of the current file and regardless of the selected text:
image.

What is the exact meaning of {ToClipBoardText}?
{CopySelectedTextToClipBoard} would be nice.

Also nice to have: Before invoking the external program mm should offer to save the {CurrentFile} if it is modified.

@RickStrahl
Copy link
Owner

Hmmm... I can duplicate the failure. I have to take a look - probably has to do with accessing the file to copy to clipboard.

{ToClipboardText} copies the current files content to the clipboard assuming the file is a text file (I check for known file types from the file extension list).

I like the idea of saving before but it definitely has to be a flag or else we'd get unexpted behavior potentially. I'll add that!

@RickStrahl
Copy link
Owner

You can give this another try with 2.7.10.3 which fixes the problem you ran into. Also added SaveBeforeActivation option,

{CopySelectedTextToClipboard} doesn't make sense to me - if you're already selecting, you might as well copy to clipboard as that's a deliberate operation.

Also updated the docs with a little more information on parameters.

@jbridgy
Copy link
Author

jbridgy commented Jan 5, 2023

{CopySelectedTextToClipboard} doesn't make sense to me - if you're already selecting, you might as well copy to clipboard as that's a deliberate operation.

Your thought is not complete. If you copied the selection manually (Ctrl-C) then you cannot apply an ExternalPrograms-command using {ToClipboardText} because it would overwrite the clipboard. So you have to switch to the external program in a different way.

{CopySelectedTextToClipboard} is more flexible than {ToClipboardText} because when you have defined an ExternalPrograms-command using {CopySelectedTextToClipboard} then you can:

  1. select the whole file (Ctrl+A)
  2. select any part of it (especially useful if the file is large)
  3. select nothing (just switch to external program)

@jbridgy
Copy link
Author

jbridgy commented Jan 7, 2023

... Also added SaveBeforeActivation option,

I think this option is inflexible and needless.
Why not simply showing the following dialog if necessary?
image
Note that the default button is Yes in this case. Markdown Monster already shows the same dialog with default button Cancel when the user tries to close a modified file (document).

Taking the suggested dialog route instead of configuring "SaveBeforeActivation" : true allows the user to recognize unintentional changes. Saving an interactively changed file is a crucial operation that should always be initiated by the user and should not be implied by another operation. As a side benefit option SaveBeforeActivation becomes superfluous.

@RickStrahl
Copy link
Owner

That dialog is specific to the close operation on shutdown so it's not a simple matter to display it on save.

However easy enough to add to the operation for the open document explicitly here by checking dirty status if the document has pending changes.

Wasn't it you who suggested the SaveOperation flag in the settings 😄

This is the better route though - I agree...

@RickStrahl RickStrahl reopened this Jan 7, 2023
@RickStrahl
Copy link
Owner

{CopySelectedTextToClipboard} is more flexible than {ToClipboardText} because when you have defined an ExternalPrograms-command using {CopySelectedTextToClipboard} then you can:

I'm not sure I understand what you mean here. If you don't specify any arguments (or other arguments) and you manually copy to clipboard (ie. select and copy (ctrl-a / ctrl-c) the text is there anyway. Taking the selection seems more intrusive as it potentially blows away another clipboard selection that you might paste into the external app. I just don't see how this gains anything if the selection has to be made manually anyway - it's one extra key (ctrl-c) to press.

@jbridgy
Copy link
Author

jbridgy commented Jan 7, 2023

This is the better route though - I agree...

Thank you.


Just for the sake of completeness:

That dialog is specific to the close operation on shutdown so it's not a simple matter to display it on save.
...
Wasn't it you who suggested the SaveOperation flag in the settings 😄

No, it did not suggest the SaveOperation flag.
In the initial post on May 1, 2022 I suggested the following:

... Before invoking the external editor mm should offer a Save Document dialog if the current file is modified, that is:

<CurrentFile> has been modified.
Do you want to save changes?
[Yes] [No] [Cancel]

Although the dialog has the title "Save Document" it is specific to the Close Document (Ctrl+W) command which is attached to the x button of each document window and is automatically invoked on shutdown (Exit command). The dialog is not connected with any Save command.

Two days ago I briefly restated my suggestion by:

Also nice to have: Before invoking the external program mm should offer to save the {CurrentFile} if it is modified.

Then you came up with the SaveOperation flag.

@jbridgy
Copy link
Author

jbridgy commented Jan 7, 2023

I'm not sure I understand what you mean here. If you don't specify any arguments (or other arguments) and you manually copy to clipboard (ie. select and copy (ctrl-a / ctrl-c) the text is there anyway.

You are right. If the external program is invoked without passing {ToClipboardText} then you have all the flexibility regarding the text to be copied. My suggestion would save only one pressing Ctrl-C.

@RickStrahl
Copy link
Owner

RickStrahl commented Jan 7, 2023

Ok so here is what I have now 😄

For the active editor document the logic now goes like this:

  • If the document exists and has no changes - just open in external
  • If the document exists and has changes: Prompt for Saving
  • If you opt not to save, the unchanged file is opened
  • If you opt to save the file with all changes is opened
  • If the document doesn't exist (ie. Untitled) - you're prompted to save
  • If you opt to save the new file is opened externally
  • If you opt to not save the editor content is copied to a temp file and opened, then deleted after 2 seconds

Here's what this looks like in code:

ci.Click += async (o, args) =>
{
    AcePosition pos = new AcePosition();
    if (model.ActiveEditor != null)
    {
        pos = await model.ActiveEditor.GetCursorPosition();
    }

    var menuItem = o as MenuItem;
    var path = menuItem.CommandParameter as string;
    var program = menuItem.Tag as ExternalProgramItem;
    program.DocumentText = Model.ActiveDocument.CurrentText;

    if (Model.ActiveDocument.IsUntitled)
    {
        bool saveToFile = await Model.Window.Dispatcher.InvokeAsync(() =>
            MessageBox.Show(Model.Window, "This 'Untitled' document " +
                                          "hasn't been saved yet.\n" +
                                          "Do you want to save it to file now?\n\n" +
                                          "If you don't save now, a temporary file " +
                                          "will be created and then deleted after a few seconds, " +
                                          "to allow opening the file externally.",
                "Save Document",
                MessageBoxButton.YesNo, MessageBoxImage.Question, MessageBoxResult.Yes) ==
            MessageBoxResult.Yes);
        if (saveToFile)
        {
            saveToFile = await Model.Window.SaveAsFile();
        }

        if (!saveToFile)   // create a temporary file and delete after 3 secs
        {
            var tfile = Path.Combine(Path.GetTempPath(),
                "_mm_untitled_" + DataUtils.GenerateUniqueId() + ".md");
            var text = await model.ActiveEditor.GetMarkdown();
            File.WriteAllText(tfile, text);
            path = tfile;
            Model.Window.Dispatcher.Delay(3000, () => File.Delete(tfile));
        }
    }
    else
    {
        await Model.Window.SaveFile(promptForSaveIfDirty: !program.SaveBeforeActivation);
    }
    
    if (program.CanExecute(path))
        program.Execute(path, pos.row + 1, pos.column + 1);
    else
        Model.Window.ShowStatusError("Couldn't execute " + program.Name + " with " + path);
};
mi.Items.Add(ci);

I ended up creating a new method on the MainWindow to handle the UI interaction for Save As operation more generically - previously this was handled only in a command which works fine as long as you don't need a result back which is required in this scenario so we can still open without saving.

I left in the SaveBeforeActivation flag which offers an override to avoid the save prompts for files that exist. If set there's no prompt - it just saves before opening externally. Personally, I prefer that as it speeds things up, although I'm Ctrl-s'aholic so I rarely have unsaved changes in my documents anyway. The flag defaults to false so the prompt is the default behavior.

This turning into a bit of Refactor 😄 Simple thing that isn't so simple once you dig into it...

@jbridgy
Copy link
Author

jbridgy commented Jan 8, 2023

The new logic looks very comfortable. Until now I missed the case where the current document is untitled (not yet existent as file). In this case I would not allow the document to be opened with an external program. This makes the code simpler and avoids the awkward situation where the temporary file is deleted while it is being used by the external program.

@RickStrahl
Copy link
Owner

Yeah I agree on the untitled behavior:

  • if Untitled
  • Ask to save
  • If saved open that file
  • Otherwise abort external execution

@jbridgy
Copy link
Author

jbridgy commented Jan 8, 2023

In mm 2.7.11.4 you forgot to adjust the highlighted text in the following dialog:
image

If you click Yes in this dialog and Cancel the subsequent "Save As" dialog then you get the following warning which is not correct:
image

@RickStrahl
Copy link
Owner

RickStrahl commented Jan 9, 2023

  • Fixed the extra dialog text. Thanks for catching that!

Can you take a look at your log file and look for this error message:

mmApp.Log("Document Save Failed:  " + filename + "  - " + Encoding?.EncodingName, ex, false,
                            LogLevels.Warning);

Curious why that would happen - I don't see that here. Could be an a location where the file can't be saved due to permissions?

@jbridgy
Copy link
Author

jbridgy commented Jan 9, 2023

The problem seems to be that mm does not check which button of the "Save As" dialog has been clicked. So it tries to save the current document even when the user clicked Cancel, that is, it tries to save "untitled" in the current directory which is the installation directory of mm. If mm is installed in %ProgramFiles%\Markdown Monster\ and is running as normal user (not as administrator) then the following error occurs:

2023-01-09 11:46:56 - Document Save Failed:  untitled  - Unicode (UTF-8)
Access to the path 'C:\Program Files\Markdown Monster\untitled' is denied.
Markdown Monster v2.7.11.5

@RickStrahl
Copy link
Owner

Argh - yup that's it. Thank you. Fixed.

I lifted that block of code from somewhere else but had to modify it heavily to fit in this simpler context and I missed the result processing from the save operation.

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

No branches or pull requests

2 participants