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 c-sharp textobjects #3494

Merged
merged 4 commits into from
Aug 27, 2022

Conversation

gbogarinb
Copy link
Contributor

This PR adds basic textobjext based navigation for c-sharp.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Jumping around in some C# files I have locally, I think we should also have function textobjects for property definitions like:

public override bool CanRead {
	get { return true; }
}

But I'm not sure if we should tag the entire property as function.around (the property_declaration node) or just the accessor declaration (accessor_declaration node)

Also constructor definitions:

public BufferPoolStream(BufferPool bufferPool) {
	if (bufferPool == null)
		throw new ArgumentNullException("bufferPool");
	_bufferPool = bufferPool;
}
(constructor_declaration body: (_) @function.inside) @function.around

@gbogarinb
Copy link
Contributor Author

gbogarinb commented Aug 22, 2022

Do you think it would be appropriate to map it like this until we have more precise navigation commands?

([
  (class_declaration body: (_) @class.inside)
  (struct_declaration body: (_) @class.inside)
  (interface_declaration body: (_) @class.inside)
  (enum_declaration body: (_) @class.inside)
  (delegate_declaration)
  (record_declaration body: (_) @class.inside)
  (record_struct_declaration body: (_) @class.inside)
  ]) @class.around

Note: To make record_struct_declaration work the grammar revision has to be updated along with its highlights.scm.

About property_declaration vs accessor_declaration, playing around, (property_declaration (_) @function.inside) @function.around feels more consistent with the rest of the commands, specially when using it with match inside/around function (mif, maf).

Class for testing.

namespace NavigationTest {
	public class ClassTest {
		public int field = 0;
		
		public int Field {
			get { return field; }
			set { field = value; }
		}

		public NavigationTest(string param1, int param2) {
			// Set field.
			field = param2;
		}

		~NavigationTest() {
		}

		/// Documentation.
		///
		/// Documentation.
		public int Method(string param1, int param2) {
			/* Multiline
				comment.
			*/
			return 0;
		}
	}
	
	public enum EnumTest {
		Item1,
		Item2
	}
	
	public interface InterfaceTest {
		public void Method();
	}
	
	public struct StructTest {
		public int field = 0;
	}

	public record RecordTest {
		public int field = 0;
	}

	public record struct RecordStructTest {
		public int field = 0;
	}
}

Let me know what you think.

runtime/queries/c-sharp/textobjects.scm Outdated Show resolved Hide resolved
runtime/queries/c-sharp/highlights.scm Outdated Show resolved Hide resolved
@gbogarinb gbogarinb marked this pull request as draft August 25, 2022 02:48
@gbogarinb gbogarinb marked this pull request as ready for review August 26, 2022 19:45
@the-mikedavis
Copy link
Member

Looks great, thank you!

@the-mikedavis the-mikedavis merged commit e066782 into helix-editor:master Aug 27, 2022
AlexanderBrevig pushed a commit to AlexanderBrevig/helix that referenced this pull request Aug 29, 2022
Co-authored-by: Gustavo Bogarín <[email protected]>
Co-authored-by: Gustavo Bogarín <[email protected]>
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
Co-authored-by: Gustavo Bogarín <[email protected]>
Co-authored-by: Gustavo Bogarín <[email protected]>
jdrst pushed a commit to jdrst/helix that referenced this pull request Sep 13, 2022
Co-authored-by: Gustavo Bogarín <[email protected]>
Co-authored-by: Gustavo Bogarín <[email protected]>
herkhinah pushed a commit to herkhinah/helix that referenced this pull request Dec 11, 2022
Co-authored-by: Gustavo Bogarín <[email protected]>
Co-authored-by: Gustavo Bogarín <[email protected]>
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.

2 participants