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

Made toggle_comments language dependent #463

Merged

Conversation

luctius
Copy link
Contributor

@luctius luctius commented Jul 17, 2021

This adds a line in the language.toml file declaring the type of single line comment to use for the specified language. This still does not support comment types like xml, nor multiline comments.

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Nice! This was definitely needed, just a few small comments.

By the way, thanks for all these contributions! Feel free to join us over at the Matrix Space (make sure to join #helix-editor:matrix.org if you're on a client that doesn't support Matrix Spaces yet) -- it's where we have discussions with other developers :)

@@ -89,7 +89,7 @@ pub struct Document {

syntax: Option<Syntax>,
// /// Corresponding language scope name. Usually `source.<lang>`.
pub(crate) language: Option<Arc<LanguageConfiguration>>,
pub language: Option<Arc<LanguageConfiguration>>,
Copy link
Member

Choose a reason for hiding this comment

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

Don't expose this, call doc.language_config() instead. We want to avoid being able to directly mutate most of the doc fields since there's more invariants.

pub fn toggle_line_comments(
doc: &Rope,
selection: &Selection,
token: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Make this Option<&str>, then you can token.unwrap_or("//") without requiring that to_owned.

@@ -3399,7 +3399,8 @@ fn hover(cx: &mut Context) {
// comments
fn toggle_comments(cx: &mut Context) {
let (view, doc) = current!(cx.editor);
let transaction = comment::toggle_line_comments(doc.text(), doc.selection(view.id));
let token = doc.language.as_ref().and_then(|l| l.comment_token.clone());
Copy link
Member

Choose a reason for hiding this comment

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

With the Option bound on the toggle_line_comments you should now be able to take a reference here without cloning.

@archseer
Copy link
Member

Looks like there are some conflicts, can you rebase?

@archseer archseer merged commit cd65a48 into helix-editor:master Jul 18, 2021
@luctius luctius deleted the features/language_dependent_comment branch July 18, 2021 18:22
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

2 participants