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

Information bar isn't navigating to the right code block #986

Closed
Saumya40-codes opened this issue Mar 5, 2024 · 21 comments · Fixed by #994
Closed

Information bar isn't navigating to the right code block #986

Saumya40-codes opened this issue Mar 5, 2024 · 21 comments · Fixed by #994
Labels
bug Something isn't working

Comments

@Saumya40-codes
Copy link
Contributor

Saumya40-codes commented Mar 5, 2024

Describe the bug

There are sections in information bar in studio which are made to navigate to particular part of the code block and view more information about that, but currently on clicking many of them, it only navigates to the end of the code block.

How to Reproduce

Steps to reproduce the issue. Attach all resources that can help us understand the issue:

AsyncAPI.Studio.-.Google.Chrome.2024-03-05.17-46-13.mp4

Expected behavior

Expected behaviour will be the navigation on the right part of the code block. For instance on clicking schema, the following part should appear
image

If it is ok, than I will like to work on this bug to solve it.
Thanks!!

@Saumya40-codes Saumya40-codes added the bug Something isn't working label Mar 5, 2024
Copy link

github-actions bot commented Mar 5, 2024

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@satishkumarsajjan
Copy link

should this be fixed? if yes, then I can work on it. assign this to me.

@Amzani
Copy link
Collaborator

Amzani commented Mar 6, 2024

Hi @Saumya40-codes please go ahead

@Amzani
Copy link
Collaborator

Amzani commented Mar 6, 2024

Hi @satishkumarsajjan @Saumya40-codes already suggested himself in the description, you can probably help to review his work.
Thanks

@Amzani Amzani moved this from Backlog to Ready in Studio - Kanban Mar 6, 2024
@satishkumarsajjan
Copy link

Oh cool then. @Saumya40-codes let me know if I can be of any help.

@Saumya40-codes
Copy link
Contributor Author

Saumya40-codes commented Mar 6, 2024

Hey, @Amzani @fmvilas
I was debugging through the code for this and found that the line

extras.document.getRangeForJsonPath(jsonPath, true);

under the function

  getRangeForJsonPath(uri: string, jsonPath: string | Array<string | number>) {
    try {

      const { documents } = documentsState.getState();

      const extras = documents[String(uri)]?.extras;

      if (extras) {
        jsonPath = Array.isArray(jsonPath) ? jsonPath : jsonPath.split('/').map(untilde);

        if (jsonPath[0] === '') jsonPath.shift();

        return extras.document.getRangeForJsonPath(jsonPath, true);
      }
    } catch (err: any) {
      return;
    }
  }

in the file src\services\parser.service.ts

is giving wrong range of starting and ending line for the given file path/jsonPath

I'm finding it hard to understand how function getRangeForJsonPath here gives the range of starting and ending part of the line.

image

I will be still looking to find the problem here but can you explain how it works internally but? maybe here or slack anywhere it is possible.

for instance when clicked to Schemas the following range appears
image

@satishkumarsajjan can you help here?

@satishkumarsajjan
Copy link

I am going through it now.

@satishkumarsajjan
Copy link

@Saumya40-codes I found out what actually is happening.

This is what we see in the apps editor:(first image)
Screenshot 2024-03-06 222606

And this is what the function is getting the positions from:(second image)
Screenshot 2024-03-06 222513

so now when i click on "Messages" in nav, I get these as start and end line numbers:(third image)
Screenshot 2024-03-06 222951

which is actually correct according to second image.
similarly when I click on "Operations" I get these line numbers: (fourth image)
Screenshot 2024-03-06 223340

Which is again correct according to second image.

@satishkumarsajjan
Copy link

In short, we are using json logic to yaml file.

@satishkumarsajjan
Copy link

My proposed solution:

  async scrollTo(jsonPointer: string | Array<string | number>, hash: string) {
    try {
      const range = this.svcs.parserSvc.getRangeForJsonPath(
        'asyncapi',
        jsonPointer
      );
      if (range) {
        await this.scrollToEditorLine(range.start.line + 1);
      }

      await this.scrollToHash(hash);
      this.emitHashChangeEvent(hash);
    } catch (e) {
      console.error(e);
    }
  }

this function which is navigation.service.ts file, should take one more argument to check the file type and then perform file specific line search logic.

This is what I could come up with. If you have better approach, let's discuss.

@Vishal2002
Copy link

Vishal2002 commented Mar 6, 2024

It is navigating to the correct code block when the document type is json .I think we have to make some changes for yaml file type because currently there are two types of files available for conversion.
Screenshot 2024-03-06 231612

@Saumya40-codes
Copy link
Contributor Author

Saumya40-codes commented Mar 6, 2024

@Vishal2002 @satishkumarsajjan thanks!
I do think there needs to be some additional checks based on the file types and currently YAML needs to be handled separately.

@Vishal2002
Copy link

@Amzani Can I also work on this issue?

@Saumya40-codes
Copy link
Contributor Author

Saumya40-codes commented Mar 7, 2024

We will need to iterate through each line of the code anyways isn't it (for .yaml files)?
Is there any better solution?

I think it will be complex implementation without knowing how maintainers want it to work like, or how existing (json) part works internally 😅

Copy link
Collaborator

Amzani commented Mar 7, 2024

@Vishal2020 sure you can coordinate with @Saumya40-codes and create a PR.

@Vishal2002
Copy link

@Saumya40-codes It would be great if we could work together on this issue. I think we need to write a separate function which will find the jsonpointer in the $ref of that document.

@Saumya40-codes
Copy link
Contributor Author

@Saumya40-codes It would be great if we could work together on this issue. I think we need to write a separate function which will find the jsonpointer in the $ref of that document.

Ok then. But I've already tried to solve this using that way, but it doesn't work in all cases.

@Saumya40-codes
Copy link
Contributor Author

Saumya40-codes commented Mar 8, 2024

Hey, @Amzani, @princerajpoot20 I've resolved the bug and following is the final output:

AsyncAPI.Studio.-.Brave.2024-03-08.18-19-57.mp4

(Have tried testing this on all examples and it works just fine)

The problem as suggest by @Vishal2002 and @satishkumarsajjan (thanks to them !) was that the existing scrollTo function for scrolling to the desirable line was only working when the file type was JSON


Now as a fix in services/navigation.service.ts file I have made following changes to scrollTo function

 async scrollTo(
    jsonPointer: string | Array<string | number>,
    hash: string,
  ) {
    try {
      const doc = this.svcs.editorSvc;
      const docType = doc.value.startsWith('asyncapi') ? 'yaml' : 'json';

      let range;
      if (docType === 'yaml') {
        range = this.svcs.parserSvc.getRangeForYamlPath('asyncapi', jsonPointer);
      } else {
        range = this.svcs.parserSvc.getRangeForJsonPath('asyncapi', jsonPointer);
      }
      
      if (range) {
        await this.scrollToEditorLine(range.start.line);
      }

      await this.scrollToHash(hash);
      this.emitHashChangeEvent(hash);
    } catch (e) {
      console.error(e);
    }
  }

Basically, i'm just checking the file type, if it is json im keeping it as it is but if it is yaml i've built another method to get the desirable range.

Here is that code(have left the comments)
(update: the code below code has been updated to the better one in the pr)

public extractKeys(obj: { [key: string]: any }, keys: Array<string>) {   \\ to get all the keys from our YAML doc
    for (const key in obj) {
      if (obj.hasOwnProperty(key) && typeof key !== 'number' && typeof key !== 'undefined') {
        keys.push(key);
        if (typeof obj[key] === 'object') {
          this.extractKeys(obj[key], keys);
        }
      }
    }
  }

  getRangeForYamlPath(uri: string, jsonPath: string | Array<string | number>) {
    try{
      const { documents } = documentsState.getState();

      const extras = documents[String(uri)]?.extras;

      if (extras) {
        jsonPath = Array.isArray(jsonPath) ? jsonPath : jsonPath.split('/').map(untilde);
        if (jsonPath[0] === '') jsonPath.shift();
        
        const doc = this.svcs.editorSvc;
        const currDoc = doc.value.split(/(\s+|\n)/).filter(Boolean);  \\ filter based on new line and space for instance if value is name: "john" \n age:18      then after this function i'll get ["name:", "john","\n","age:","18:]

        const yamlDoc: object = yaml.load(doc.value) as object; \\ convert our string (taken from this.svcs.editorSvc.value to a yaml object)   Had to use **js-yaml** library for this if any alternative is possible then please suggest

        const keys: Array<string> = [];
        this.extractKeys(yamlDoc, keys); \\ getting all the keys

        \\ below function just loops through the doc and will either add "\n" or the key only

        const propertyNames = currDoc.map((line:string,index:number, res: string[]): string  | null => { 
          
          if (line.includes("\n")) {
            let times = (line.match(/\r\n/g) || []).length;
            return "\n".repeat(times);
          }

          const match = line.match(/^\s*([^:]+)\s*:/);
          if(match && keys.includes(match[1])) {
            return String(match[1]);
          }
          else {
            return null;
          }
        }).filter(Boolean);

        if(!propertyNames) {
          return null;
        }

        let lineCnt = 0;
        let idx = 0; \\ will keep track of the path we want 

       \\ just a simple iteration to get the desired result
        for (let i = 0; i < propertyNames.length; i++) {
          if (propertyNames && String(propertyNames[i]).includes(String(jsonPath[idx]))) {

            let tempIdx = i+1;
            let isRef = false;

            while(tempIdx < propertyNames.length) {    \\ this are for the case when ref is used as the keyword we want might be used here
              if(String(propertyNames[tempIdx]).includes("$ref")) {
                isRef = true;
                break;
              }
              else if(!String(propertyNames[tempIdx]).includes("\n")) {
                break;
              }
              tempIdx++;
            }

            if(isRef){
              continue;
            }
            idx++;

            if (idx >= jsonPath.length) {
              break;
            }
          } else if(propertyNames && String(propertyNames[i]).includes("\n")) {
            lineCnt+= (propertyNames[i]?.split("") || []).length;
          }
        }

        return {
            start: { line: lineCnt+1, character: 0 }, 
            end: { line: lineCnt+1, character: String(propertyNames[lineCnt]).length } 
        };
      }
    }
    catch (err) {
      return err;
    }
  }

Might have overdone the things but I tried to keep it as optimized as possible.
Any inputs @Vishal2002 @satishkumarsajjan ???

@Vishal2002
Copy link

@Saumya40-codes It looks nice 👍👍

@satishkumarsajjan
Copy link

If it works then raise a PR. Members will review it. The app already uses stoplight lib to parse and get the path. It also does it for yaml, you could have used it and that would have been easy. But if this works then i see no issue. Nice work.

@Saumya40-codes
Copy link
Contributor Author

If it works then raise a PR. Members will review it. The app already uses stoplight lib to parse and get the path. It also does it for yaml, you could have used it and that would have been easy. But if this works then i see no issue. Nice work.

Oh I wasn't fully aware about that!!
Will migrate the changes accordingly then.
Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants