Add descriptor details to the command language #148

Open
Vectornaut wants to merge 27 commits from Vectornaut/dyna3:details into main
Member

I'll add more details to this description later. I'm posting it as-is because I don't want writing the description to delay the opening of the pull request.

Summary

The branch to be merged improves the command language introduced in pull request #138, adding all the features we need to write parsimonious loading scripts for our current test assemblies. The central new feature is the descriptor details syntax, which is based on the idea of "creation attributes" from our design discussions.

Features

In addition to the descriptor details syntax, the branch to be merged introduces the following language features:

  • Identifier assignment.
  • Element attributes, including label, color, and ghost.
    • The syntax for attributes applies to all descriptors, but element descriptors are the only ones that have attributes implemented for them so far.
  • Access to engine-specific element representations.

Loading scripts

The branch to be merged includes a loading script for each test assembly, proving that it includes enough language features to do that. The test assembly menu now has a second entry for each test assembly, which loads the assembly using a script. I've done a few visual and numerical checks to verify that the loading scripts produce the same assemblies as the original loading functions, although these checks aren't particularly thorough or reproducible.

*I'll add more details to this description later. I'm posting it as-is because I don't want writing the description to delay the opening of the pull request.* ## Summary The branch to be merged improves the command language introduced in pull request #138, adding all the features we need to write parsimonious loading scripts for our current test assemblies. The central new feature is the *descriptor details* syntax, which is based on the idea of ["creation attributes"](wiki/Command-language#creation-attributes) from our design discussions. ## Features In addition to the *descriptor details* syntax, the branch to be merged introduces the following language features: - [Identifier assignment](wiki/Command-language#assigning-identifiers). - Element attributes, including *label*, *color*, and *ghost*. - The syntax for attributes applies to all descriptors, but element descriptors are the only ones that have attributes implemented for them so far. - Access to engine-specific element representations. ## Loading scripts The branch to be merged includes a loading script for each test assembly, proving that it includes enough language features to do that. The test assembly menu now has a second entry for each test assembly, which loads the assembly using a script. I've done a few visual and numerical checks to verify that the loading scripts produce the same assemblies as the original loading functions, although these checks aren't particularly thorough or reproducible.
Implement the element label attribute as an example.
In the process, simplify vertical alignment in outline items.
In particular, stop the test assembly loading effect from reacting to
element identifier changes.
Originally, this code was accidentally placed between ReceiveRegulator
implementation blocks.
This will help us interpret details in descriptor arguments.
This allows redundant parentheses, which we probably want, since they're
allowed in every language I can think of.
In the process, introduce floating point array literals.
A new statement begins on each new non-indented line. This rule is kind
of kludgy, but it's good enough for now.
Some signals (like `DVector` signals) seem to get tracked when you
format them, while others (like `String` signals) don't. This commit
makes the untracking explicit for signals of the latter kind.
The new loading scripts are based on assembly exports from another
branch. Numbers in the old loading scripts have been reformatted for
consistency.
Add loading scripts for dynamic test assemblies
All checks were successful
/ test (pull_request) Successful in 3m48s
f5e824bf63
These are the test assemblies that trigger non-trivial realizations.
@ -163,0 +164,4 @@
f,
"{}{}",
self.label().get_clone_untracked(),
self.representation().get_clone_untracked(),
Owner

Please now can you put the id in here as the primary identifier? That's how we want to use it -- id should be the basic/standard/common way to refer to an entity; the label is just for showing the entity, and it will be quite common for assemblies to have multiple entities with the same label. Thanks.

Please now can you put the id in here as the primary identifier? That's how we want to use it -- id should be the basic/standard/common way to refer to an entity; the label is just for showing the entity, and it will be quite common for assemblies to have multiple entities with the same label. Thanks.
Author
Member

Done in commit 23a7382. I don't like how the label and identifier have the opposite ordering as in the outline view, but ultimately it's up to you.

Done in commit 23a7382. I don't like how the label and identifier have the opposite ordering as in the outline view, but ultimately it's up to you.
Owner

Hmm, I am definitely not trying for there to be any difference in ordering. What I am trying for is the id to be the primary identifier of an entity throughout dyna3, and for the label to (a) be a secondary characteristic specifying one aspect of the decoration of an entity, much akin to "color," and (b) ultimately default to the id if no label other than that has ever been specified. So in the end if our canonical name-list for points ends up being A,B,C,... then I would expect the first point created with no id specified have id "A" and thereby an implicit label "A" and so on. I would expect to see in the graphic display, assuming that label display is on for this point, the text "A" plotted next to this point, and in the outline view, that this point is simply designated as A; there is no need to indicate an id and label separately, because the id is always shown and no label has ever been set.

Then maybe I create a bunch more points, but then decide I want the point with id "I" to have label "centroid" so I change it, however that's currently done. I would then expect that (a) assuming label display is on for this point in the graphic display, the text plotted next to the point changes from "I" to "centroid", and (b) the primary designation of this point in an outline view remains "I", but that there might well be some indication that the label has been set, so the outline might change from showing just I to showing (say) I "centroid" or something like that.

I guess we've never really had a conversation about the use/semantics of labels and ids along these lines before, for which I apologize. So I realize switching to their roles as described in this comment might be out of scope for this PR, and if so, should be captured in a separate issue. If so, just let me know and I will file an issue laying out the desired roles of id and label, and in the meantime, you should do only as much in this PR insofar as moving toward the goals of that issue as is consistent with the scope of this PR. Thanks!

Hmm, I am definitely not trying for there to be any difference in ordering. What I am trying for is the id to be the primary identifier of an entity throughout dyna3, and for the label to (a) be a secondary characteristic specifying one aspect of the _decoration_ of an entity, much akin to "color," and (b) ultimately default to the id if no label other than that has ever been specified. So in the end if our canonical name-list for points ends up being A,B,C,... then I would expect the first point created with no id specified have id "A" and thereby an implicit label "A" and so on. I would expect to see in the graphic display, assuming that label display is on for this point, the text "A" plotted next to this point, and in the outline view, that this point is simply designated as `A`; there is no need to indicate an id and label separately, because the id is always shown and no label has ever been set. Then maybe I create a bunch more points, but then decide I want the point with id "I" to have label "centroid" so I change it, however that's currently done. I would then expect that (a) assuming label display is on for this point in the graphic display, the text plotted next to the point changes from "I" to "centroid", and (b) the primary designation of this point in an outline view remains "I", but that there might well be some indication that the label has been set, so the outline might change from showing just `I` to showing (say) `I "centroid"` or something like that. I guess we've never really had a conversation about the use/semantics of labels and ids along these lines before, for which I apologize. So I realize switching to their roles as described in this comment might be out of scope for this PR, and if so, should be captured in a separate issue. If so, just let me know and I will file an issue laying out the desired roles of id and label, and in the meantime, you should do only as much in this PR insofar as moving toward the goals of that issue as is consistent with the scope of this PR. Thanks!
@ -197,2 +202,3 @@
pub representation: Signal<DVector<f64>>,
pub color: Signal<ElementColor>,
pub ghost: Signal<bool>,
pub representation: Signal<DVector<f64>>,
Owner

Is the new order of fields in this struct important? Would generally avoid just permuting without a specific motive..

Is the new order of fields in this struct important? Would generally avoid just permuting without a specific motive..
Author
Member

When I was organizing action.rs, putting the label, color, and ghost descriptors together led to the most readable code, because these are all element attributes, interpreted in similar ways. Since the representation descriptor is an engine data attribute, the code for interpreting it looks pretty different.

I didn't want to break with my convention of putting interpreter code sections related to Element fields in the same order as those fields, so I decided to re-order the fields. My original motivation for putting the representation field between the label and color fields feels pretty weak (and I barely remember it), so I think there's little cost to changing it.

When I was organizing `action.rs`, putting the label, color, and ghost descriptors together led to the most readable code, because these are all element attributes, interpreted in similar ways. Since the representation descriptor is an engine data attribute, the code for interpreting it looks pretty different. I didn't want to break with my convention of putting interpreter code sections related to `Element` fields in the same order as those fields, so I decided to re-order the fields. My original motivation for putting the representation field between the label and color fields feels pretty weak (and I barely remember it), so I think there's little cost to changing it.
glen marked this conversation as resolved
@ -299,2 +308,3 @@
pub representation: Signal<DVector<f64>>,
pub color: Signal<ElementColor>,
pub ghost: Signal<bool>,
pub representation: Signal<DVector<f64>>,
Owner

Same comment on permuting fields.

Same comment on permuting fields.
Author
Member

Same response as above.

Same response as [above](https://code.studioinfinity.org/StudioInfinity/dyna3/pulls/148#issuecomment-3896).
Owner

OK, resolving both of these comments per your explanation. However: do we want to have some section of these structures commented as the "attributes" so that it will be clear where future attributes should go? There will certainly be numerous others in the long run: size and or shape for points, maybe ultimately texture for spheres, maybe "hidden" state for both, etc.

OK, resolving both of these comments per your explanation. However: do we want to have some section of these structures commented as the "attributes" so that it will be clear where future attributes should go? There will certainly be numerous others in the long run: size and or shape for points, maybe ultimately texture for spheres, maybe "hidden" state for both, etc.
glen marked this conversation as resolved
@ -883,0 +908,4 @@
|elts_by_id| !elts_by_id.contains_key(&id_new)
);
if can_rename {
console_log!("Can rename"); /* DEBUG */
Owner

Should such debugging console logs be removed in a main-line commit? I generally think so...

Should such debugging console logs be removed in a main-line commit? I generally think so...
Author
Member

Yes, I think so too. Done in commit 43eb341!

Yes, I think so too. Done in commit 43eb341!
glen marked this conversation as resolved
@ -880,6 +899,42 @@ impl Assembly {
}
}
pub fn try_rename_element(
Owner

Name of this function? We are using "name" in a different way in dyna3, if I understand correctly, so that the name of a point element is "Point", and that name cannot be changed. Shouldn't this be something like try_change_id ?

Name of this function? We are using "name" in a different way in dyna3, if I understand correctly, so that the name of a point element is "Point", and that name cannot be changed. Shouldn't this be something like `try_change_id` ?
Author
Member

I see what you mean. I don't envision making "name" part of our user-facing terminology, because to me it feels like it could equally well refer to an identifier or a label, but we do use it internally in the Named trait, which we apply to any structure we want to give an associated NAME constant to. I agree that a method name like one of these would clarify the connection to the id method of Element:

  • try_change_id
  • try_change_element_id
  • try_set_id
  • try_set_element_id

Any preferences?

I see what you mean. I don't envision making "name" part of our user-facing terminology, because to me it feels like it could equally well refer to an identifier or a label, but we do use it internally in the `Named` trait, which we apply to any structure we want to give an associated `NAME` constant to. I agree that a method name like one of these would clarify the connection to the `id` method of `Element`: - `try_change_id` - `try_change_element_id` - `try_set_id` - `try_set_element_id` Any preferences?
Owner

I had suggested "(try_)change_id" as being closest to "rename". I don't know that we need to mention it operates on elements (unless we need a similar method for regulators?) since in a call it will have the element as an argument, so it will be clear it is operating on an element. So I guess my preference is "try_change_id" but I am ok with "try_set_id" as well, and if there is ever a need to have a similar operation for something other than elements, we could put the word "element" back in the name at that time.

I had suggested "(try_)change_id" as being closest to "rename". I don't know that we need to mention it operates on elements (unless we need a similar method for regulators?) since in a call it will have the element as an argument, so it will be clear it is operating on an element. So I guess my preference is "try_change_id" but I am ok with "try_set_id" as well, and if there is ever a need to have a similar operation for something other than elements, we could put the word "_element_" back in the name at that time.
Owner

Oh, and if we end up with a design by which renaming always succeeds by some mechanism, then I suppose we can remove the "try_" part from the name...

Oh, and if we end up with a design by which renaming always succeeds by some mechanism, then I suppose we can remove the "try_" part from the name...
Author
Member

Done, in commit 9f64bd8.

Done, in commit 9f64bd8.
@ -883,0 +906,4 @@
) -> bool {
let can_rename = self.elements_by_id.with_untracked(
|elts_by_id| !elts_by_id.contains_key(&id_new)
);
Owner

Are the naming requests coming from "user interaction" or are they somehow internally generated? If the former, the most common behavior I have seen in other dyngeo systems is to change the id of the other element with the colliding id to an algorithmically generated variant, and accept the id change on the current element. Perhaps this is a point that warrants further discussion?

Are the naming requests coming from "user interaction" or are they somehow internally generated? If the former, the most common behavior I have seen in other dyngeo systems is to change the id of the other element with the colliding id to an algorithmically generated variant, and accept the id change on the current element. Perhaps this is a point that warrants further discussion?
Author
Member

Right now, the only way to change an element's identifier is through the command prompt, although I imagine we'll want to provide ways to change it through the outline view and other views too.

If the former, the most common behavior I have seen in other dyngeo systems is to change the id of the other element with the colliding id to an algorithmically generated variant, and accept the id change on the current element.

This behavior sounds very confusing to me, so I'd like to play with some examples if possible. Neither of the systems I tried behaved this way.

  • In the web version of GeoGebra, when I create points with inputs A = (1, 0) and B = (0, 1) and then try to rename the second point by changing its input to A = (0, 1), I get an error in the second input cell: "This label is already in use A."
  • In the web version of Geometry Expressions, I can't figure out how to rename points at all.
Right now, the only way to change an element's identifier is through the command prompt, although I imagine we'll want to provide ways to change it through the outline view and other views too. > If the former, the most common behavior I have seen in other dyngeo systems is to change the id of the other element with the colliding id to an algorithmically generated variant, and accept the id change on the current element. This behavior sounds very confusing to me, so I'd like to play with some examples if possible. Neither of the systems I tried behaved this way. - In the web version of GeoGebra, when I create points with inputs `A = (1, 0)` and `B = (0, 1)` and then try to rename the second point by changing its input to `A = (0, 1)`, I get an error in the second input cell: "This label is already in use A." - In the web version of Geometry Expressions, I can't figure out how to rename points at all.
Owner

In the web version of GeoGebra, when I create points with inputs A = (1, 0) and B = (0, 1) and then try to rename the second point by changing its input to A = (0, 1), I get an error in the second input cell: "This label is already in use A."

Yes, that is what happens if you try to have the "algebra input" to GeoGebra have two lines with identical identifiers. Our command language is a bit different, so we will have to think about what should happen if we have two different "A is..." commands in a row.

However, in the web version of geogebra, if you create points with inputs A = (1, 0) and B = (0, 1) and then right click on point B in the graphic display and select settings, and then in the "Name" field delete "B" and enter "A" and hit return, then "B" is renamed to "A" and "A" is renamed to "A₁".

> In the web version of GeoGebra, when I create points with inputs A = (1, 0) and B = (0, 1) and then try to rename the second point by changing its input to A = (0, 1), I get an error in the second input cell: "This label is already in use A." Yes, that is what happens if you try to have the "algebra input" to GeoGebra have two lines with identical identifiers. Our command language is a bit different, so we will have to think about what should happen if we have two different "A is..." commands in a row. However, in the web version of geogebra, if you create points with inputs A = (1, 0) and B = (0, 1) and then right click on point B in the graphic display and select settings, and then in the "Name" field delete "B" and enter "A" and hit return, then "B" is renamed to "A" and "A" is renamed to "A₁".
Owner

I should take back the "most common behavior" comment. I just tried three projects with recent code activity: Dr.Geo, Kig, and FluxGeo. They all simply allow one to give two points the same "name." However, the first two have no command language, so the concept of "name" is essentially what we're calling "label" -- there's no need/opportunity to refer to an object by name. In FluxGeo, though, there is a command interpreter, and renaming a second point to A produces the frustrating situation that there is no way to refer to that second point -- using A in the command interpreter continues to refer only to A. (The interpreter does report a unique interal object id, akin to our serial, on each object creation, but I don't see how to refer to an object by this id...) I would categorize this situation with respect to naming in FluxGeo as either "poorly designed" or "buggy".

I should take back the "most common behavior" comment. I just tried three projects with recent code activity: [Dr.Geo](https://github.com/Dynamic-Book/DrGeo), [Kig](https://github.com/KDE/kig), and [FluxGeo](https://fluxgeo.org/app). They all simply allow one to give two points the same "name." However, the first two have no command language, so the concept of "name" is essentially what we're calling "label" -- there's no need/opportunity to refer to an object by name. In FluxGeo, though, there is a command interpreter, and renaming a second point to A produces the frustrating situation that there is no way to refer to that second point -- using A in the command interpreter continues to refer only to A. (The interpreter does report a unique interal object id, akin to our serial, on each object creation, but I don't see how to refer to an object by this id...) I would categorize this situation with respect to naming in FluxGeo as either "poorly designed" or "buggy".
Owner

The difficulty with just having a command from the person making the construction to rename a point to A fail with "'A' is already in use" is that often one is making a construction with specific names in mind for the points. But maybe it's not convenient to create them in the order A, B, C,... So the auto-assigned names get off and you have to rename some of them. So you focus on one you know the name of, and issue a rename command. There may be a lot of points already, and you didn't realize the name was in use, but you know you want this point to be named A. If it fails, you have to go and find the point that currently happens to be named A, and change its name to something. Well, the natural thing to change it to is the name that point is supposed to have. But maybe that name is in use, too! Etc, etc. And if the whole thing happens to be in a cycle, it leaves the human with the burden of breaking the cycle somewhere with a temporary name, and ending up doing n+1 renamings when there were only n things in the cycle. It's a recipe for frustration.

So I would say when the person wants to rename (i.e., change the id) of a point, and specifically instructs dyna3 to do so, then that id change has to succeed. But we also have to preserve the invariant that ids are distinct. So I don't see any way to avoid geogebra's behavior of altering the id of the entity that had the id that the person now wants to assign elsewhere. I am certainly open to other suggestions, though, so we should discuss.

The difficulty with just having a command from the person making the construction to rename a point to A fail with "'A' is already in use" is that often one is making a construction with specific names in mind for the points. But maybe it's not convenient to create them in the order A, B, C,... So the auto-assigned names get off and you have to rename some of them. So you focus on one you know the name of, and issue a rename command. There may be a lot of points already, and you didn't realize the name was in use, but you know you want _this_ point to be named A. If it fails, you have to go and find the point that currently happens to be named A, and change its name to something. Well, the natural thing to change it to is the name _that_ point is supposed to have. But maybe that name is in use, too! Etc, etc. And if the whole thing happens to be in a cycle, it leaves the human with the burden of breaking the cycle somewhere with a temporary name, and ending up doing n+1 renamings when there were only n things in the cycle. It's a recipe for frustration. So I would say when the person wants to rename (i.e., change the id) of a point, and specifically instructs dyna3 to do so, then that id change has to succeed. But we also have to preserve the invariant that ids are distinct. So I don't see any way to avoid geogebra's behavior of altering the id of the entity that had the id that the person now wants to assign elsewhere. I am certainly open to other suggestions, though, so we should discuss.
Author
Member

However, in the web version of geogebra, if you create points with inputs A = (1, 0) and B = (0, 1) and then right click on point B in the graphic display and select settings, and then in the "Name" field delete "B" and enter "A" and hit return, then "B" is renamed to "A" and "A" is renamed to "A₁".

Aha! Thanks for explaining.

In our setup, the thing that makes this behavior most confusing to me is to imagine reading a command history like this.

a is Point: Label = "Ali"
a is Point: Label = "Anita"
a is Point: Label = "Artur"
a is Point: Label = "Aya"

Given this history, what state could we be in?

Current behavior

If identifier assignment fails when you try to assign an identifier that's already in use, it's easy to understand what state we've ended up in. The first command has succeeded, as long as the identifier a wasn't in use already, and the subsequent commands have all failed. If the first command succeeded, we've created one point:

a (Ali)

This result matches what I'd naïvely expect the first command to do.

GeoGebra's behavior

With GeoGebra's behavior, we need to understand the renaming system and know the full state of the assembly to work out what state we've ended up in. If none of the a-series identifiers were in use already, we end up with these points:

a1 (Ali)
a2 (Anita)
a3 (Artur)
a (Aya)

This isn't a result I could've guessed before trying it. If any of the a-series were in use, the result gets even more chaotic. For example, if we already had a triangle

a (A)
b (B)
c (C)

we now have

a1 (A)
b (B)
c (C)
a2 (Ali)
a3 (Anita)
a4 (Artur)
a (Aya)

If we already had a line segment

a1 (A₁)
a2 (A₂)

we now have

a1 (A₁)
a2 (A₂)
a3 (Ali)
a4 (Anita)
a5 (Artur)
a (Aya)

This may not be a big deal in GeoGebra, where there's no scripting system or command history, and thus no sense of tension between what the commands look like they do and what they actually do. In dyna3, though, it seems like we expect the command system to be pretty central, so I feel like it's more important for commands to have straightforward results.

> However, in the web version of geogebra, if you create points with inputs A = (1, 0) and B = (0, 1) and then right click on point B in the graphic display and select settings, and then in the "Name" field delete "B" and enter "A" and hit return, then "B" is renamed to "A" and "A" is renamed to "A₁". Aha! Thanks for explaining. In our setup, the thing that makes this behavior most confusing to me is to imagine reading a command history like this. ``` a is Point: Label = "Ali" a is Point: Label = "Anita" a is Point: Label = "Artur" a is Point: Label = "Aya" ``` Given this history, what state could we be in? #### Current behavior If identifier assignment fails when you try to assign an identifier that's already in use, it's easy to understand what state we've ended up in. The first command has succeeded, as long as the identifier `a` wasn't in use already, and the subsequent commands have all failed. If the first command succeeded, we've created one point: ``` a (Ali) ``` This result matches what I'd naïvely expect the first command to do. #### GeoGebra's behavior With GeoGebra's behavior, we need to understand the renaming system _and_ know the full state of the assembly to work out what state we've ended up in. If none of the `a`-series identifiers were in use already, we end up with these points: ``` a1 (Ali) a2 (Anita) a3 (Artur) a (Aya) ``` This isn't a result I could've guessed before trying it. If any of the `a`-series were in use, the result gets even more chaotic. For example, if we already had a triangle ``` a (A) b (B) c (C) ``` we now have ``` a1 (A) b (B) c (C) ``` ``` a2 (Ali) a3 (Anita) a4 (Artur) a (Aya) ``` If we already had a line segment ``` a1 (A₁) a2 (A₂) ``` we now have ``` a1 (A₁) a2 (A₂) ``` ``` a3 (Ali) a4 (Anita) a5 (Artur) a (Aya) ``` This may not be a big deal in GeoGebra, where there's no scripting system or command history, and thus no sense of tension between what the commands look like they do and what they actually do. In dyna3, though, it seems like we expect the command system to be pretty central, so I feel like it's more important for commands to have straightforward results.
@ -28,1 +21,3 @@
},
let script_result = batch(
|| run_script(&state, &value.get_clone())
);
Owner

Is the idea that now what I type all at once in the command prompt might be multiple commands, and that's why this is now run_script instead of command.execute?

Is the idea that now what I type all at once in the command prompt might be multiple commands, and that's why this is now run_script instead of command.execute?
Author
Member

You can still only enter one statement at a time in the command prompt—not as an intentional decision, but just because statements are separated by newlines, which the HTML <input type="text"> element doesn't accept.

I replaced action::interpret with action::run_script because I only wanted to have one syntax rule that tries to match all the way to the end of the string it's given (the script rule), and I only wanted the action module to expose one function that interprets a code string.

I don't see a need for a function that tries to parse exactly one statement, and returns an error if it finds multiple statements. It wouldn't be too hard to introduce one if we want it, though.

You can still only enter one statement at a time in the command prompt—not as an intentional decision, but just because statements are separated by newlines, which the HTML `<input type="text">` element doesn't accept. I replaced `action::interpret` with `action::run_script` because I only wanted to have one syntax rule that tries to match all the way to the end of the string it's given (the `script` rule), and I only wanted the action module to expose one function that interprets a code string. I don't see a need for a function that tries to parse exactly one statement, and returns an error if it finds multiple statements. It wouldn't be too hard to introduce one if we want it, though.
Owner

No, it wasn't my intention to have a different special function for single-command strings. I guess I was imagining that run_script was a loop over something like command_execute and so wondering why the change here in a context that can only have one command. But I infer from your comment that's not really how run_script is built and there really isn't any analogue of a single command_execute anymore, in which case this is fine as it is. Resolving.

No, it wasn't my intention to have a different special function for single-command strings. I guess I was imagining that run_script was a loop over something like command_execute and so wondering why the change here in a context that can only have one command. But I infer from your comment that's not really how run_script is built and there really isn't any analogue of a single command_execute anymore, in which case this is fine as it is. Resolving.
glen marked this conversation as resolved
glen left a comment

OK, I've gone through the code. Nice work! It is nice that this PR is much smaller than the action system PR. I haven't actually tried to run the new code, but I think there are a few things to chew on in the comments I made in the meantime. I do see the increase in adding things untracked, so that's good.

OK, I've gone through the code. Nice work! It is nice that this PR is much smaller than the action system PR. I haven't actually tried to run the new code, but I think there are a few things to chew on in the comments I made in the meantime. I do see the increase in adding things untracked, so that's good.
Add identifier to element Display formatting
All checks were successful
/ test (pull_request) Successful in 3m45s
23a7382ed9
Drop debug logs from identifier assignment
All checks were successful
/ test (pull_request) Successful in 3m48s
43eb341e2c
Rename identifier change method
Some checks failed
/ test (pull_request) Failing after 1m45s
1d30378fb6
Vectornaut force-pushed details from 1d30378fb6
Some checks failed
/ test (pull_request) Failing after 1m45s
to 9f64bd8a6f
All checks were successful
/ test (pull_request) Successful in 3m44s
2026-06-03 20:41:15 +00:00
Compare
All checks were successful
/ test (pull_request) Successful in 3m44s
Required
Details
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u details:Vectornaut-details
git switch Vectornaut-details
Sign in to join this conversation.
No description provided.