lejsue / symbols-navigator

Symbols navigator for Atom.io
MIT License
13 stars 3 forks source link

function with a default $arg string value prevents symbol from showing up #29

Closed farinspace closed 3 years ago

farinspace commented 5 years ago

Something like this will not be recognized and displayed in the symbol list:

// does not work
protected function foobar( $a, $b = NULL, $c = 'baz' )

// does not work
protected function foobar( $a, $b = NULL, $c = "baz" )

// works (but is incorrect)
protected function foobar( $a, $b = NULL, $c = baz )

// works
protected function foobar( $a, $b = NULL )

Seems related to #20 but I am using "symbols-navigator 1.8.1"

j2bbayle commented 4 years ago

Same here, using v1.9.0 and Python. This is a really annoying bug, since most Python scripts use many methods with string-defaulted arguments.

Maybe something that can help:

# this works
def write(self, output="output.h5"):

# and this does not
def write(self, output='output.h5'):

# nor this
def write(self, output="output.h5", mode='x'):

# this is incorrect Python syntax, but to make my case, this does not work
def write(self, 'output'):

It seems that as soon a line contains simple quotes, symbol-navigator can't parse it correctly.

j2bbayle commented 3 years ago

Mmm… That's actually an easy one: in "lib/tag-generator.js", we escape special characters in the list of tags generated by ctags. This includes simple quotes ('), double quotes (") and backslashes (). It turns out that it's invalid to escape simple quotes inside a double-quoted string in JSON, and the JSON parser fails silently for this tag -- which ends up with the symbol being ignored.

There is an easy fix: in "lib/tag-generator.js", remove line 28, so that the function parseTagLine(line) now reads

  parseTagLine(line) {
    let tagLine = line;
    if (tagLine.length > 0 && tagLine[0] === '{') {
      let sections = {};
      try {
        // escape the signatures to allow correct parsing
        const searchString = '"signature":"(';
        let startpos = tagLine.indexOf(searchString);
        if (startpos !== -1) {
          startpos += searchString.length;
          const endpos = tagLine.lastIndexOf('"');
          tagLine = tagLine.substring(0, startpos) +
            tagLine.substring(startpos, endpos)
              .replace(/\\/g, '\\\\')
              .replace(/"/g, '\\"') +
            tagLine.substring(endpos);
        }
        sections = JSON.parse(tagLine);
      } catch (exception) {
        console.log(tagLine)
        console.error(exception)
        return null;
      }

      if (sections.name == null || sections.line == null) {
        return null;
      }

      const tag = {
        position: new Point(parseInt(sections.line, 10) - 1),
        name: sections.name,
        type: sections.kind,
        parent: null,
        access: null,
        signature: null,
      };

      if (sections.scopeKind != null && sections.scopeKind !== '') {
        tag.parent = `${sections.scopeKind}:${sections.scope}`;
      }

      if (sections.access != null && sections.access !== '-') {
        tag.access = sections.access;
      }

      if (sections.signature != null && sections.signature !== '-') {
        tag.signature = sections.signature;
      }

      return tag;
    }

    return null;
  }

@lejsue can you quickly push this change?

lejsue commented 3 years ago

@j2bbayle Thanks for the solution, I just published the new version. Please let me know if that is ok. And really happy to see this package is still be using in someone's atom editor.

j2bbayle commented 3 years ago

Thanks a lot @lejsue, that seems to work just fine!

I am using a lot this package. I tried many, and this is only one that allows a nice integration with docking in Atom's various spaces. Thanks a lot for that!