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/Change: Adaptive server name in prompt for ement-create-room #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rayes0
Copy link

@rayes0 rayes0 commented Apr 15, 2022

Quick change to show the homeserver for the selected session instead of matrix.org when creating rooms.

@alphapapa
Copy link
Owner

I appreciate this idea. It might be worth doing. But first:

  1. The copyright assignment still needs to be done. (Technically you could contribute up to 15 lines of changes to the project without doing so, but I don't want to have to keep track of that sort of thing.)
  2. The indentation in this patch is very inconsistent. Please use (setf indent-tabs-mode nil) and be sure to indent code properly before submitting (I recommend using aggressive-indent-mode for Lisp code, as it completely solves this problem).

@alphapapa alphapapa force-pushed the master branch 2 times, most recently from 12c2abf to 2e072ef Compare April 15, 2022 15:30
@alphapapa
Copy link
Owner

Well, I do appreciate your fixing the whitespace, but you also changed the whitespace in the entire file rather than just the lines you changed. (Having worked on Ement in a different Emacs config for a while, which I neglected to set indent-tabs-mode in, it still has some tabs in it--I'll need to fix that in a separate commit sometime.) Please update the patch to only have the lines you've added.

@rayes0
Copy link
Author

rayes0 commented Apr 15, 2022

Should be good now, didn't realize you didn't me to fix the whole file :)

@alphapapa
Copy link
Owner

Thank you. :)

Comment on lines +389 to +391
(replace-regexp-in-string "\\<https?://\\|/$" ""
(ement-server-uri-prefix (ement-session-server session)))
"\"): "))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just use ement-server-name here, right?

@alphapapa alphapapa force-pushed the master branch 2 times, most recently from 7d2e0f2 to c0b922b Compare May 14, 2023 04:24
@alphapapa alphapapa added enhancement New feature or request discussion labels Jul 9, 2023
@alphapapa alphapapa added this to the Future milestone Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants