Closed amyjko closed 5 months ago
Hi Amy, could you point me some direction on where to start on this issue?
Yep!
email: 'ajko@uw.edu'
. It'll give a conflict that it seems to be personally identifiable information. There's an option there to say "No, it's not", and then the warning goes away. That's what this issue will create, but for duplicate names.DuplicateName.ts
, and the Resolution
type in Conflict.ts
. Everything you need to build is in DuplicateName.ts
, returning a Resolution
that revises the program to remove the duplicate name.That should get you started.
Thanks @xmzeng00! Here's some feedback:
name, name: 1
and you'll see it). The only part of the design proposal that doesn't exist it the prompt and button to remove.Hi Amy, not sure if you would get a notification on adding design specifications. Jessi, and I updated the design specification draft2 here yesterday. Thanks for your help :))
I don't get notifications when you edit the issue. You should always tag and comment to get someone's attention in GitHub; that's the only way to generate a notification.
I'm a little unclear on what your draft 2 proposal is proposing. We already show a conflict when there are duplicate names. This issue is about allowing creators to resolve the conflict by clicking a button to remove the name. Your proposal seems to be redundant with the conflict message already shown. Can you clarify?
Hey Amy, we have updated the design spec! Can you take a look at it? We are also considering the case that users may want to keep the duplicate names in languages for the purpose of international development collaboration. Is this something we should also consider, or say there's no such case?
Thanks for the design proposal @xmzeng00! A bit of feedback:
Hi Amy, thank you for your feedback! Relevant changes have been made to the post.
Okay, I'm marking this buildable. Congrats on the approved design!
It's the end of Winter! Please provide an update on this issue, including:
If you do plan to continue work on it, carry on. Otherwise, thank you for your contributions!
I will continue to work on it!
No reply from @xmzeng00, unassigning.
Hi, Amy! Could you give me some pointers on how to implement the design proposal?
Yep! The key concept to understand is that the the Conflict
type (defined in Conflict.ts
) has a function getConflictingNodes
that can optionally return a list of Resolution
, each a little object that has a description of the resolution and function that returns a revised Project
that resolves the conflict. The DuplicateName
conflict (defined in DuplicateName.ts
) just means that a Bind
has multiple names that are the same, so a Resolution
might be returned that replaces the original Bind
with a Bind
that does not have the duplicate name.
The files to modify for this would be:
DuplicateName.ts
to return a Resolution
that revises the projectNodeTexts.ts
node > Bind > conflict > DuplicateName
to define a description of the resolution, per the designen-US.json
, es-MX.json
, zh-CN.json
) and the template example.json
)Hi Amy! Thank you for the pointers! We've fixed and tested the issue for the case where a declaration has a duplicate name (or more than one duplicate name). However, we were confused about the case when there are duplicate names on separate lines, like in the following image: We didn't see anything about this in the design specification, so we wanted to know whether this is included in the scope of this issue. Should there be a conflict resolution for this case as well?
Ah, good catch; this issue was really about a single bind expression with duplicate names. The case of two separate bind statements can't really be resolved automatically, since there's no way to fix it without the creator making a decision about which one to keep. So consider that out of scope.
You mentioned that you implemented this, but I don't see a pull request for this issue. If you're ready for review, submit a PR.
What's the problem?
When a declaration has a duplicate name, creators have to manually remove it to resolve the warning:
name,name: 5
What's the design idea?
Add a conflict resolution for
DuplicateName
that removes one of the duplicates.Who benefits?
Anyone looking for a quick fix to the problem.
Design specification
Draft 2
Adding a prompt on the middle console, showing the user, "There is a duplicate name, do you want to change it to something else". Once the user changes the duplicated one, the warning goes away.
Draft 3
Normal case
Solution
When there are duplicate names, there will be a prompt with a button on the middle console showing "There are duplicate names, do you want to remove it? Note: the duplicate names will never be beneficial to your program." Once the user clicks the check button, one duplicated name will be removed, and the warning will go away.
Prototype
Edge Case 1 -- duplicate name with different languages
Solution 1
Previligae the default language setting:
Prototype
Solution 2
Provide choices to users:
Prototype
Draft 4
Normal case
Solution
When there are duplicate names, there will be a prompt with a button on the middle console showing "There are duplicate names, do you want to remove it?" Once the user clicks the check button, one duplicated name will be removed, and the warning will go away.
Prototype