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.

Sign up for GitHub

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

Jump to bottom

Add code snippets to JSON and XML in builtin provider #254

Closed
pranavgaikwad opened this issue Jul 12, 2023 · 1 comment · Fixed by #412
Closed

Add code snippets to JSON and XML in builtin provider #254

pranavgaikwad opened this issue Jul 12, 2023 · 1 comment · Fixed by #412
Assignees
@fabianvf
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone
Demo F

Comments

@pranavgaikwad
Copy link
Contributor

It'd be great to have code locations in JSON and XML capabilities too similar to what I did in #253
@pranavgaikwad pranavgaikwad mentioned this issue Jul 12, 2023
Merged
@pranavgaikwad pranavgaikwad added this to the Demo F milestone Jul 12, 2023
@pranavgaikwad pranavgaikwad moved this to 🆕 New in Konveyor Analyzers Jul 14, 2023
@fabianvf fabianvf self-assigned this Jul 27, 2023
@dymurray dymurray moved this to 📋 Backlog in Planning Sep 15, 2023
@pranavgaikwad pranavgaikwad moved this from 📋 Backlog to 🔖 Ready in Planning Oct 12, 2023
@djzager djzager mentioned this issue Oct 12, 2023
Closed
1 task
@djzager djzager added kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Oct 13, 2023
@djzager
Copy link
Member

djzager commented Oct 13, 2023

Motivation for calling this a feature and not a bug fix revolves around the fact that the jsonquery and xmlquery libraries we are currently using do not track the location of nodes in the xml/json documents they are working with. Finding the location of a node matching some xml/json query is not immediately obvious.

We could:

  1. Write our own XML/JSON parser to keep track of nodes, implement location tracking, and get code snippets that way.
  2. Submit patches to the xmlquery and json query deps.

It's not immediately obvious that this is a huge ask from our user base. Currently rating this as an important long-term feature.

@pranavgaikwad pranavgaikwad removed the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Nov 7, 2023
@pranavgaikwad pranavgaikwad mentioned this issue Nov 13, 2023
Merged
pranavgaikwad added a commit that referenced this issue Nov 16, 2023
@pranavgaikwad
✨ add dependency location resolver and get code locations for XML, JS…
39ed5f5 
…ON files (#412)

Fixes #410 
Fixes #254 

There were two choices to do this: 

1. Add a new optional field lineNumber to Dep type, and let the
providers populate it at the time of dependency discovery. This is
straight-forward and probably preferred, but requires a change in the
output API.
2. Add a new interface like Snipper, that providers can implement for
deps to get locations for a certain dependency.

I chose No. 2 because output API change not needed. But I am not fan of
it as its complicated & perf penalty. Asking for your inputs on the
approach. If we do go with 2, I think we should we should still remove
it in later versions in favor of a new optional field on Dep. Even if we
did add it now, it would be non-intrusive.

---------

Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
@github-project-automation github-project-automation bot moved this from 🔖 Ready to ✅ Done in Planning Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fabianvf fabianvf

Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
Planning
Status: ✅ Done
Konveyor Analyzers
Status: 🆕 New
Milestone
Demo F
3 participants
@djzager @fabianvf @pranavgaikwad