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

Add option to edit current document #225

Closed
wants to merge 13 commits into from

Conversation

shitchell
Copy link
Contributor

Implements #182

Allows the user to edit the current document by typing 'e'. If $EDITOR is set, a temporary file is created with the contents of the current document, and the specified editor is used to open it. If no changes are made when the editor exits, nothing happens. If changes are made, then:

if the document is a local file, the edits are written to that file
if the document is stashed, a new stash is created with the edits, and the old stash is deleted.

It would be ideal to simply update the contents of a stash, but the charm API doesn't currently allow this. Deleting the selected stash has the side effect that, when the user returns to the list of files and tries to re-open the same document, the stash won't load. Restarting glow solves this and shows the newly created, updated stash.

charmbracelet/charm#13 creates a SetMarkdownBody() method to resolve this minor issue.

Calling `showStatusMessage(note)` always displays "Stashed!". This fixes the
method to allow the intuitive behavior and display the parameter passed as a
status message.
Added an option to edit the opened document when the user types 'e'. Searches
$PATH first for the command specified by $EDITOR and then for a set of
pre-defined command line text editors. Addresses:
charmbracelet#182
@shitchell
Copy link
Contributor Author

shitchell commented Dec 18, 2020

So I'm realizing now which parts of this should be (non-)blocking. Launching the editor needs to be blocking. But once the edits are made, saving the local file / stashing the changes should be done asynchronously. Working on that now

Should complete this feature for now. Asynchronous methods added for stashing or writing edits. Also tracks the ID of edited stashes better so that future editing / revisiting a file is no longer buggy.
@shitchell
Copy link
Contributor Author

Alright, as far as I can tell, this is in a pretty good state. Found no bugs while testing. Code is much more organized. Stuff that should block blocks and stuff that shouldn't doesn't.

@meowgorithm
Copy link
Member

meowgorithm commented Dec 18, 2020

Thank you for the PR!

Just a heads up that we're working on an update to Glow that will most likely conflict with this, though we're happy to look at those conflicts on our end. The holidays are also upon us so it may be a little bit before we give this PR a proper review.

@shitchell
Copy link
Contributor Author

Baha, no worries. I'm happy to look over it, too. If there's a fork or branch where the update is being worked on, I could look over adjusting my code to work with it

@meowgorithm
Copy link
Member

meowgorithm commented Dec 19, 2020

Cool, we'll push something up early next week. We may contribute to your branch as well.

@shitchell
Copy link
Contributor Author

shitchell commented Dec 20, 2020

So I've been testing this out with various editors. It behaves oddly with GUI editors.

  • The file remains displayed while the editor is launched.
    • This might not be a bad thing! If/when Pager live preview #148 is implemented, it might be nice to have a live preview of the document while you're editing it. However, this likely won't work out of the box; we'd probably have to re-work some of the current code to have a live preview while a GUI editor is in use.
  • The control isn't necessarily immediately returned to glow after you're finished editing. For example, if I use EDITOR=subl glow when I'm already using Sublime Text to edit other files... when I'm finished editing the glow document, I don't exit Sublime since I'm still using it for other stuff. Since glow is waiting for the editor to exit, I then have to either quit Sublime and interrupt my other work or kill glow and lose my edits.

A couple possible solutions:

  1. Use fsnotify to detect when the temporary file being used for edits is written to. Copy those changes to the local file or upload them to the stashed file as appropriate for every write event.
  2. If we can detect whether the editor is GUI based or console based, make GUI file editing non-blocking. Instead, display a message like "File opened in external editor. Press 'enter' to continue when finished.".

I'm already using fsnotify to implement #148, so that shouldn't be too bad.

However, I haven't the slightest idea how to detect if the editor is GUI or console based, but intuition says... while nothing is technically "impossible" when programming, this probably leans towards more complex than it's worth. If that's the case, I currently don't have a good idea for returning control back to glow when finished editing.

@i3d
Copy link

i3d commented Apr 30, 2021

Having a direct editing feature would be awesome!

@kinduff
Copy link

kinduff commented Dec 7, 2021

@shitchell curious if you tried using EDITOR=subl --wait in order for the editor to return once the file is saved/closed. Sorry for the necrobump.

@maaslalani
Copy link
Member

maaslalani commented Jan 6, 2023

Hey @shitchell, thank you so much for this PR!

Open in editor was added to Glow recently in:

You can now press e on any local document to edit it.

@maaslalani maaslalani closed this Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants