-
Notifications
You must be signed in to change notification settings - Fork 116
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
added variable to make doom-modeline optional #45
Conversation
this closes issue #44 Signed-off-by: Erik Lundstedt <erik@lundstedt.it>
Co-authored-by: Mathieu Marques <mathieumarques78@gmail.com>
Co-authored-by: Mathieu Marques <mathieumarques78@gmail.com>
Choice of mode line customization is a pretty big one, on the order of choosing your preferred font and color theme. I'm starting to wonder if we should configure the mode line somewhere else, like in a separate module that can be included only if you want mode line customizations. That would make it easier to have multiple What do you think? |
I don't know I was planning on making a pr with the telephone-line modeline config I've got where you run a function to use different configurations/defaults and the user get to start the modeline themselves as you need to do the config before starting the modeline with |
Actually |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current commit history is a bit of a mess. While the intend is fine, can you please clean it up? Given the contents of your changes, you could squash them into a single commit.
(customize-set-variable 'doom-modeline-bar-width 6) | ||
(customize-set-variable 'doom-modeline-minor-modes t) | ||
(customize-set-variable 'doom-modeline-buffer-file-name-style 'truncate-except-project) | ||
(defcustom rational-ui-use-doom-modeline t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't concur with setting a variable that controls an optional feature to t
by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't concur with setting a variable that controls an optional feature to t by default.
thats to not break existing configs
i could change that i guess
I am not sure on how to do that as ive only ever used git(gitlab) for my own code where messy history isnt a problem |
It is always an issue, it's just that GitHub (and GitLab) will optionally let you squash commits while merging once the PR is accepted. That being said, squashing yourself is the conventional thing to do and it helps codeowners review changes more easily without carrying the minor details we talked about here To squash your commits into one, you can pull your branch locally as per latest, and if you're using Magit, ri from the status buffer is probably the easiest |
the thing now is that there have been changes made afterwards im gonna try |
this closes issue #44
Signed-off-by: Erik Lundstedt erik@lundstedt.it