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

Show Only Diff option should show "context" #3640

Closed
KrzysztofCwalina opened this issue Jul 13, 2022 · 7 comments
Closed

Show Only Diff option should show "context" #3640

KrzysztofCwalina opened this issue Jul 13, 2022 · 7 comments
Assignees
Labels
APIView Central-EngSys This issue is owned by the Engineering System team. Epic
Milestone

Comments

@KrzysztofCwalina
Copy link
Member

KrzysztofCwalina commented Jul 13, 2022

Show Only Diff is a great option that is unfortunately totally unusable today. If shows very small slices of the full listing (just the changed members) which makes it impossible to tell what type the members are in and what other members are in the same type.

I propose that this option simply filters out all types that don't have changes, but otherwise shows full types with changes and their namespaces.

Update 6/14:

  • Active development and testing will keep happening until June 20th
  • By June 20th Dozie will share the link of an APIView environment which will be stable for you to test.
    • If critical changes need to be made, they will be communicated over Teams
  • Documentation will be improved and shared by the same date
  • Unless there is critical feedback that needs to be addressed right away, we will deploy to Production on June 27th
  • Rethink migration strategy for the other parsers
@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 13, 2022
@KrzysztofCwalina KrzysztofCwalina moved this to 🆕 New in ApiView Jul 13, 2022
@maririos maririos added APIView Central-EngSys This issue is owned by the Engineering System team. labels Jul 13, 2022
@azure-sdk azure-sdk moved this to 🤔Triage in Azure SDK EngSys 🚢🎉 Jul 13, 2022
@maririos maririos removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 13, 2022
@praveenkuttappan praveenkuttappan moved this from 🆕 New to 📋 Backlog in ApiView Aug 1, 2022
@JonathanGiles
Copy link
Member

@praveenkuttappan @chidozieononiwu I suspect for this to work we would need to improve the language parsers to discern in the JSON output between lines that should never be filtered out in diff view (i.e. class declarations), and the rest of lines that can be removed if they are not diffed. Would you agree? If so, did you want to make a proposal that we could implement in the parser for each language?

@praveenkuttappan
Copy link
Member

That's true. APIView doesn't have full context of parent components which makes it hard to find parent context lines to show. Adding a context like Level 0, Level 1, Level2 etc to show tree structure will help us find the context and show entire context with it's siblings.

@JonathanGiles
Copy link
Member

@praveenkuttappan What would be more useful for APIView? It seems we have two different approaches:

  1. Annotate the lines that are non-hidable
  2. Annotate all lines with their level (I wonder if at that point we can remove all code in our parsers related to adding indentation whitespace).

@praveenkuttappan
Copy link
Member

  1. Currently tokens are created as a flat list of tokens with line separator. So there is no hierarchy context.
  2. Annotating lines with their level number is better option.

@maririos
Copy link
Member

Feedback from @KrzysztofCwalina : "The way I would address the problem is by changing "show diff only" to "show types with diffs only", i.e. if any members had diffs, the whole type with these members is shown."

@maririos maririos added the Epic label Sep 12, 2023
@maririos maririos added this to the 2024-01 milestone Sep 12, 2023
@maririos maririos removed the Epic label Sep 13, 2023
@MrJustinB MrJustinB changed the title Show Only Diff options should show "context" Show Only Diff option should show "context" Sep 20, 2023
@tjprescott
Copy link
Member

Related: #7375

@chidozieononiwu chidozieononiwu moved this from 📋 Backlog to 🏗 In progress in ApiView May 9, 2024
@maririos maririos added the Epic label Jul 29, 2024
@maririos
Copy link
Member

This has been completed as part of the work on #5897 and #8174 .
Once those epics are stable, other languages can onboard the Tree Token parser and use the Diff with context feature

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in ApiView Jul 31, 2024
@github-project-automation github-project-automation bot moved this from 🤔 Triage to 🎊 Closed in Azure SDK EngSys 🚢🎉 Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIView Central-EngSys This issue is owned by the Engineering System team. Epic
Projects
Status: ✅ Done
Archived in project
Development

No branches or pull requests

7 participants