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

hover show detail feature + minor bug fix #1

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

Conversation

vkameswaran
Copy link

@vkameswaran vkameswaran commented May 2, 2024

Shows details on hover: gives class name + variable values for instances and assigned value for variables. Minor bug fix for when bracket is in a word.

Greptile Summary

  • Introduced hover feature to display class names and variable values
  • Fixed bug for handling brackets within words
  • Added hoverProvider capability in package.json
  • New utility function remove_special_character for text cleanup

greptile-apps[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
greptile-apps-local[bot]

This comment was marked as resolved.

greptile-apps[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
greptile-apps-local[bot]

This comment was marked as resolved.

greptile-apps[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
greptile-apps[bot]

This comment was marked as resolved.

greptile-apps-local[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
greptile-apps-local[bot]

This comment was marked as resolved.

greptile-apps[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
greptile-apps[bot]

This comment was marked as resolved.

greptile-apps-local[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 6, 2024
@vkameswaran vkameswaran reopened this May 6, 2024
greptile-apps[bot]

This comment was marked as resolved.

greptile-apps-local[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 7, 2024
@vkameswaran vkameswaran reopened this May 7, 2024
greptile-apps-local[bot]

This comment was marked as resolved.

greptile-apps[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 7, 2024
@vkameswaran vkameswaran reopened this May 7, 2024
greptile-apps-local[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran closed this May 7, 2024
@vkameswaran vkameswaran reopened this May 7, 2024
greptile-apps[bot]

This comment was marked as resolved.

greptile-apps[bot]

This comment was marked as resolved.

greptile-apps[bot]

This comment was marked as resolved.

greptile-apps-local[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran reopened this May 23, 2024
Copy link

@greptile-apps-local greptile-apps-local bot left a comment

Choose a reason for hiding this comment

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

Pull Request Summary

  • Hover Feature: Introduces a hover feature that displays class names and variable values for instances, as well as assigned values for variables.
  • Bug Fix: Fixes a minor bug related to handling brackets within words.
  • Utility Function: Adds a new utility function remove_special_character to support the hover feature by cleaning up class names or variable values.
  • Extension Configuration: Updates package.json to add the hoverProvider capability, enabling the new hover information feature.

Notes

  • The new utility function remove_special_character could be reused in other parts of the codebase where similar string cleaning is required.

@LSP_SERVER.feature(lsp.TEXT_DOCUMENT_HOVER)
def hover_definition(params: lsp.HoverParams) -> Optional[lsp.Hover]:
if not params.text_document.uri.startswith("file://"):
return lsp.CompletionList(is_incomplete=False, items=[])

Choose a reason for hiding this comment

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

Returning lsp.CompletionList in a hover handler is incorrect. It should return None instead.

greptile-apps[bot]

This comment was marked as resolved.

@vkameswaran vkameswaran reopened this May 23, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Pull Request Summary

  • Hover Feature Enhancement: Introduces a new hover feature that displays class names and variable values for instances, and assigned values for variables, enhancing the user experience with more detailed information.
  • Bug Fix: Addresses a minor bug related to handling brackets within words, improving overall text processing.
  • Utility Function Addition: Adds remove_special_character in lsp_utils.py to clean up words by removing specific special characters, supporting the hover feature and bug fix.
  • Extension Capability Update: Updates package.json to include hoverProvider capability, integrating the new hover feature into the extension.

Notes:

  • The new utility function remove_special_character could be reused in other text processing features for consistency.

@LSP_SERVER.feature(lsp.TEXT_DOCUMENT_HOVER)
def hover_definition(params: lsp.HoverParams) -> Optional[lsp.Hover]:
if not params.text_document.uri.startswith("file://"):
return lsp.CompletionList(is_incomplete=False, items=[])
Copy link

Choose a reason for hiding this comment

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

Returning lsp.CompletionList for hover feature is incorrect. It should return None instead.

@@ -39,7 +39,8 @@
"virtualWorkspaces": {
"supported": false,
"description": "Virtual Workspaces are not supported with atopile."
}
},
"hoverProvider" : "true"
Copy link

Choose a reason for hiding this comment

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

The value for hoverProvider should be a boolean. Change "true" to true.

pass
else:
# TODO: deal with assignments made to super
output_str += f"**class**: {str(atopile.address.get_name(instance.supers[0].address))}\n\n"

Choose a reason for hiding this comment

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

Consider handling cases where instance.supers might be empty.

Comment on lines +71 to +73
def remove_special_character(word: str) -> str:
return "".join(e for e in word if e not in "(){}")

Choose a reason for hiding this comment

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

Consider adding a docstring to explain the purpose and usage of this function.

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.

1 participant