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

Respect "short" option in Symbol.get_name() #1170

Merged
merged 2 commits into from
Nov 17, 2024

Conversation

rocky
Copy link
Member

@rocky rocky commented Nov 17, 2024

And start a unit test for mathics.core.symbols.

(Much more testing of that class would be nice, but that's left for some other time.)

@rocky rocky requested a review from mmatera November 17, 2024 11:40
@@ -575,7 +575,7 @@ def get_attributes(self, definitions):
return definitions.get_attributes(self.name)

def get_name(self, short=False) -> str:
return self.name
return self.name.split("`")[-1] if short else self.name
Copy link
Contributor

Choose a reason for hiding this comment

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

@rocky, notice that in the code, we use the method Definitions.shorten_name() which keeps track of the current $Context and $ContextPath to decide whether the name of a symbol should or should not be shortened.

Copy link
Member Author

@rocky rocky Nov 17, 2024

Choose a reason for hiding this comment

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

Ok. I am not totally certain how to use this here or if this is the right thing, user interface-wise.

It might be that in the debugger TraceActivate and TraceDebug do some sort of context-based searching. But on the other hand , it might also be that in a debugger ContextPath's change so you want the looseness. When the test is done might be in a totally different context from the context that set up the trace.

I will add some comments and possibly TODO's in the debugger code so this idea not lost and made more in the forefront.

But I also note that if there is a parameter named "short=False" offered, it is wrong to have it not work.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I also note that if there is a parameter named "short=False" offered, it is wrong to have it not work.

Yep, I totally agree. Let's merge this when you feel it is ready.

Copy link
Member Author

@rocky rocky Nov 17, 2024

Choose a reason for hiding this comment

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

@mmatera See 4fc61c7 and Mathics3/Mathics3-trepan@49b03ca

If things get merged and there are additional issues, comments, or concerns, they will be addressed in a follow-up.

about the use of $Context and $ContextPath in normal operation
@rocky rocky merged commit 15b8092 into master Nov 17, 2024
13 checks passed
@rocky rocky deleted the respect-short-in-Symbol-get_name branch November 17, 2024 16:16
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