Open in editor

[runar-indico]

When viewing a file with less, one can press v to open the file in the editor. Adding this feature to glow would be great.

[lepasq]

Hi! I'd like to work on this issue.

This shouldn't take too long, so I'll try to make a PR within the next days.

[muesli]

@lepasq Awesome, looking forward to that!

[lepasq]

I've worked on the feature for a bit, and so far I encountered one problem:

Executing any editor will crash glow right after exiting the editor.
You can try this yourself by running exec nvim or exec nano in your terminal emulator, and quitting nvim/nano.

I couldn't find a workaround to this issue yet.
A StackOverflow user explained the problem here.

Shall I post my gitdiff in the meantime, such that others can have a look at my code and try to solve it?

Error message in terminal: Error: read /dev/stdin: resource temporarily unavailable

[meowgorithm]

@lepasq Yes, please do! You could also post a link to your fork as well if you prefer.

[lepasq]

@meowgorithm I just pushed my changes to the pager-edit-key branch of my fork in this commit.

[lepasq]

Note that the code won't work on Windows yet, since I'm running the editor by launching its binary in /bin.

[wantyapps]

You can check the value of the "$EDITOR" variable, and launch it.

[lepasq]

You can check the value of the "$EDITOR" variable, and launch it.

Fair point. This is what the method currently looks like:

func findEditor() (string, error) {
	if os.Getenv("EDITOR") != "" {
		return os.Getenv("EDITOR"), nil
	}
	editors := []string{"nvim", "vim", "vi", "nano", "gedit"}
	for _, editor := range editors {
		if _, err := exec.LookPath("/bin/" + editor); err == nil {
			return "/bin/" + editor, nil
		}
	}
	return "", fmt.Errorf("Couldn't find an editor on this system.")
}

Do we want to remove the second part, in which we check if the user has an editor inside /bin, in case that the user did not set the environment variable?
In that case, we would just rely on the $EDITOR environment variable, and either ignore the v key action or return some error feedback, if $EDITOR is undefined.

[wantyapps]

Hi! I want to make some changes in the code you sent earlier. Did you push the code to charmbracelet/glow, or is it still in your fork?

[wantyapps]

Could not fork a fork. Lol! The change I wanted to make was to add the values ["code", "subl"] to the "editors" array. Code is for VSCode, and Subl is the command for Sublime Text.

[shitchell]

Fair point. This is what the method currently looks like:

func findEditor() (string, error) {
	if os.Getenv("EDITOR") != "" {
		return os.Getenv("EDITOR"), nil
	}
	editors := []string{"nvim", "vim", "vi", "nano", "gedit"}
	for _, editor := range editors {
		if _, err := exec.LookPath("/bin/" + editor); err == nil {
			return "/bin/" + editor, nil
		}
	}
	return "", fmt.Errorf("Couldn't find an editor on this system.")
}

Do we want to remove the second part, in which we check if the user has an editor inside /bin, in case that the user did not set the environment variable?
In that case, we would just rely on the $EDITOR environment variable, and either ignore the v key action or return some error feedback, if $EDITOR is undefined.

Looking only in /bin/ is not a very great way of looking for executables. There are a billion other folders where those binaries might live, namely the directories in the user's $PATH. And since LookPath already checks in the user's $PATH, I'm really not sure why you're adding "/bin/" to the name. I would try something more like this:

// Returns an editor name, editor path, or error
func getEditor() (string, string, error) {
        var editor_name string
        var editor_path string
        var editor_err error

        editors := []string{"micro", "nano", "nvim", "vim", "vi", "gedit"}

        // If $EDITOR is set, prepend it to the list of editors we'll search for
        if os.Getenv("EDITOR") != "" {
                editors = append([]string{os.Getenv("EDITOR")}, editors...)
        }

        // By default, the error should be that no command has been found
        editor_err = fmt.Errorf("No editor found\n")

        // Search for the editors, stopping after the first one is found
        for i := 0; i < len(editors) && editor_path == ""; i++ {
                editor_name = editors[i]

                // Look for the editor in $PATH
                path, err := exec.LookPath(editor_name);

                if err == nil {
                        // If it was found, store the path (exiting the loop)...
                        editor_path = path
                        // ...and set the error to nil
                        editor_err = nil
                } else {
                        // Delete the name so that none is returned if we don't
                        // find an editor
                        editor_name = ""
                }                          
        }

        return editor_name, editor_path, editor_err
}

Just because the user has $EDITOR set doesn't mean it's installed. We still have to search for it, just like the other predefined editors. So we just prepend it to our editors array to give it priority. Then we loop over that array while i < len(editors) and editor_path == "", that is, until we run out of editors to look for or we find an editor, whichever comes first. Then we spit back the editor path.

The reason we set up our loop that way is because it's typically best to (1) avoid break statements and (2) have only 1 return statement in your code. Forces you to keep your logic much better organized.

[shitchell]

I went ahead and added the option :D Just waiting on them to review / approve the PR

[shitchell]

Could not fork a fork. Lol! The change I wanted to make was to add the values ["code", "subl"] to the "editors" array. Code is for VSCode, and Subl is the command for Sublime Text.

What priority would/should code and subl get? I would err towards putting them at the end of the list, but then I'd wager they'd never get used. I didn't read your comment before making the PR, and I'm more than happy to see those editors added to the list, but it's probably preferable to just set $EDITOR to one of those. If you sometimes want to use, say, vim and sometimes want to use subl, create an alias like alias subglow="EDITOR=subl glow"

[wantyapps]

OK! When they approve the PR, I will update glow, and try it!

BTW: Thank you for responding to my comments. I am programming now for 1 year and love to help.
and, just saying, I am under 14 years old.
Good luck with the PR!

[shitchell]

No problem :) If you're not familiar with how to set the EDITOR variable, you put something like this in your .bashrc file:

export EDITOR="subl"

Then, whenever you use something like glow or any command that uses a text editor, it should default to using Sublime (or whatever you set for your editor).

[shitchell]

Well if anyone wants to use this, you can clone: https://github.com/shitchell/glow/tree/editor

The guys in charge said (1) they're pretty busy and probably won't be able to review it for a while and (2) an update coming next week will probably conflict with this :P So I'm probably going to keep using this branch for a bit and then re-work it after they roll out this update. They also said they might look at re-working it to help integrate it with their update? So no idea when it'll be available haha. But it's way more useful than I imagined, so I just thought I'd drop this comment here.

[a-marionette]

Any update on this? Would be way more practical to use Glow if I could create new documents via Glow and edit them as well.

[shitchell]

Any update on this? Would be way more practical to use Glow if I could create new documents via Glow and edit them as well.

I've got a version that was working December of last year. But then they rolled out an update that they said would break it. Honestly, I haven't even checked if their update conflicts with the code I wrote; this is my last semester of college, and I've been focused on that.

Buuut the semester ends in a week, so I'll take a look at this again afterwards! In the mean time, you can feel free to clone/build the branch I created. You'll basically roll back to the version from December 2020, but I find this feature far more useful than anything included in their most recent update :3

[meowgorithm]

Hey all. Just a note that we're planning on implementing this one, however to do it properly we'll most likely make some changes upstream in Bubble Tea (see charmbracelet/bubbletea#171).

The feature itself will most likely be implemented in Glow's beta branch.

[gimbling-away]

Bump

[managedkaos]

I'm looking forward to this one!

I edit a lot of Markdown..... a loooooooot of Markdown 😩 😂

I'm loving the interaction with Glow on the CLI for viewing nested folders with Markdown files. Honestly, Glow is amazing for that! 🤩

However, when I'm viewing files, I often have the need to drop into the file for quick edit. Right now I can't do that in Glow so I keep notes of the edits I want to make and get to them outside of my Glow session. Would be cool to keep it all in one session and flow.

Also, if Glow had editing, I might just use it as a full blown Markdown IDE! 😎

Thanks to everyone working on this feature. Just wanted you to know that it will really mean a lot to the way that I work so I am thanking you all in advance. 🏆 🙏🏿

[decentral1se]

Nice! As an addition, it'd be nice to see an editable: false config option to stop folks editing files. In the case of read-only files.

[ryuheechul]

what an exciting future edition!
I would also vote for key e instead of v or both or ability to set which key to open editor via config or a flag.