Skip to content

Altered Naming Convention for Duplicates#640

Open
Ammelll wants to merge 10 commits into
mjansen4857:mainfrom
Ammelll:main
Open

Altered Naming Convention for Duplicates#640
Ammelll wants to merge 10 commits into
mjansen4857:mainfrom
Ammelll:main

Conversation

@Ammelll

@Ammelll Ammelll commented Mar 13, 2024

Copy link
Copy Markdown

In my experience the current naming convention of "Copy of {AutoName}" is very inconvenient, primarily when sorting the Path/Auto names alphabetically as duplicating an auto will send it to a random place in the stack instead of right next to the Path/Auto being duplicated. I believe the changes I've proposed would address this.

@github-actions github-actions Bot added the GUI Changes to the PathPlanner GUI label Mar 13, 2024
@rjbell4

rjbell4 commented Mar 29, 2024

Copy link
Copy Markdown

I concur that something that appends a suffix would be preferred over something the prepends a prefix, as it would result in less scrolling when creating new paths or autos by duplicating something that exists and then modifying it.

@mjansen4857

Copy link
Copy Markdown
Owner

Just checking in on the status of this as it still has failing tests and needs to be formatted.

You can format by running dart format . in the root of the project directory.

@Ammelll

Ammelll commented Dec 13, 2024

Copy link
Copy Markdown
Author

The only test this was failing was test's for duplicate auto and duplicate path names (expected). How do you suggest I make these tests pass given I'm changing their function?

@mjansen4857

mjansen4857 commented Dec 18, 2024

Copy link
Copy Markdown
Owner

It looks like the tests are failing because the actual naming logic is not working as expected. For example, when duplicating Example Path (1) it expects to see Example Path (2) but doesn't.

String source = pathName.substring(pathName.length - 3);
while (pathNames.contains(pathName)) {
pathName = 'Copy of $pathName';
if (exp.hasMatch(source)) {

@mjansen4857 mjansen4857 Dec 18, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm assuming tests are failing because you're matching the regex against the substring with the (x) removed. I think this should be changed to match against the entire path name and then only do the substring if it has a match. Same thing for the autos.

EDIT: nvm I read this wrong, i thought the substring removed the (x) but it only contains that

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

When fixing this, could you also add on to the duplication tests to duplicate them a third time. i.e. check for Example Path (3) just to be extra sure the increment works.

@Ammelll Ammelll requested a review from mjansen4857 February 8, 2025 23:10
@Ammelll

Ammelll commented Feb 26, 2025

Copy link
Copy Markdown
Author

@mjansen4857 anything holding this up?

@Ammelll

Ammelll commented Jul 13, 2025

Copy link
Copy Markdown
Author

@mjansen4857 bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GUI Changes to the PathPlanner GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants