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

Support composing messages in an external editor #155

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Conversation

capgelka
Copy link
Contributor

@capgelka capgelka commented Sep 10, 2023

I found it not that difficult to implement this feature that I wanted.

If there are some issues with the current implementation, I would be glad to fix them.

PR adds a new command :editor that opens $EDITOR in the current window or in a new one (if $EDITOR is a GUI one), using a new temporary file, the message from this file will be sent immediately after leaving the editor.
Also, current text from a buffer (if any) will be used as the initial content of the file.

@Fabrice-Bernes
Copy link

after saving the file, the message will be sent immediately.

I think it's better to send the message only after leaving the editor, no? Sometimes I will save what I have and keep writing.

In any case, thanks for adding this feature.

@capgelka
Copy link
Contributor Author

You are right! Hopefully, the crate I used works exactly the way you described, just didn't realize that at first, because tested it only with :wq. So I have fixed the description.

@Fabrice-Bernes
Copy link

Hey, I compiled from your commit but :editor returns Failed command: invalid command: editor.

@capgelka
Copy link
Contributor Author

capgelka commented Sep 12, 2023

Are you sure that you are running the version from my commit? Maybe you have compiled the release but running an old debug version or vice versa.

If you are on Linux nm target/debug/iamb | grep iamb_editor (or release) should succeed. If not, you are running the wrong binary.

@ulyssa ulyssa changed the title Implement #126 Compose messages in external editor Support composing messages in an external editor Sep 12, 2023
@ulyssa ulyssa merged commit 0565b6e into ulyssa:main Sep 13, 2023
0 of 3 checks passed
@ulyssa
Copy link
Owner

ulyssa commented Sep 13, 2023

@capgelka This looks great, thank you for implementing this! I added some code to force a redraw of the screen afterwards, since at least for me locally I had leftover characters on the screen after the subprocess exited.

@Fabrice-Bernes
Copy link

I hope it's ok to ask about this now that the commit has been merged,
but i'd like to know what I did wrong when trying @capgelka 's commit.

  • I'm on linux,
  • ran git clone "https://github.com/ulyssa/iamb" && cd iamb
  • then git fetch origin pull/155/head:MASTER
  • git checkout MASTER
  • See capgelka's changes there
  • cargo install --force --locked iamb
  • cd ~/.cargo/bin && ./iamb

Thanks in advance

@capgelka
Copy link
Contributor Author

I also hope that it's ok to have such a conversation, though probably better to use stack overflow for such questions anyway.
To install a package from the current directory one has to use . as an argument, so you have just reinstalled iamb from crates.io

@ulyssa
Copy link
Owner

ulyssa commented Sep 13, 2023

Yep, continuing commenting here is fine!

As @capgelka said, the positional argument to cargo install is the name of a crate on crates.io to install. You can use cargo install --path /path/to/iamb/repo/ to install from the directory. You can also do cargo build --release, and copy the binary from ./target/release/iamb.

@Fabrice-Bernes
Copy link

I should have noticed when the installation section of the README doesn't mention cloning the repo...
Anyways, i tried again, now with --path and it works just fine.

Thank you both for the explanation and sorry for dragging this conversation.

@ulyssa ulyssa added this to the v0.0.9 milestone Feb 29, 2024
@ulyssa ulyssa mentioned this pull request Mar 29, 2024
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.

3 participants