Skip to content
This repository was archived by the owner on Jul 9, 2024. It is now read-only.

bug fixes, #16 and handling of exports_objects.lines #23

Conversation

clausreinke
Copy link

  • spurious \r in tags files on windows #16: on windows, with CRLF fileendings, tag addresses end with
    \r, causing tag lookup to fail
    remove those '\r$' in regexify
    TODO: line ending variations should be handled properly when
    splitting the file into lines in the first place
  • for some reason, tagVarRefsId may be called on IDENTIFIER nodes
    without fourth argument (why?), for instance when processing
    lib/jsctags/traits.js;
    guard handling of exports_objects.lines to keep jsctags from
    falling over
    TODO: check whether this hides a bug
  • in getTags, exports_objects.lines needs to be addressed with
    tag name, without trailing '-', to avoid undefined lineno

- mozilla#16: on windows, with CRLF fileendings, tag addresses end with
       \r, causing tag lookup to fail
       remove those '\r$' in regexify
  TODO: line ending variations should be handled properly when
        splitting the file into lines in the first place
- for some reason, tagVarRefsId may be called on IDENTIFIER nodes
  without fourth argument (why?), for instance when processing
  lib/jsctags/traits.js;
  guard handling of exports_objects.lines to keep jsctags from
  falling over
  TODO: check whether this hides a bug
- in getTags, exports_objects.lines needs to be addressed with
  tag name, without trailing '-', to avoid undefined lineno
@dimvar
Copy link

dimvar commented Apr 14, 2012

@clausreinke tagVarRefs is called without a 4th argument most of the time. It's only called with a 4th argument in commonJS mode, when we find a DOT expression for exports, eg, exports.foo.

But the guard you added is right, when exports appears by itself, there was a bug. I just pushed a fix for that, thanks!

The slice you're wondering about (?slice(1)?) is for the special properties "-number" and "-string".

@dimvar dimvar closed this Apr 14, 2012
@clausreinke
Copy link
Author

Could you please refer to the commits that fix the issues covered in this ticket? You merely mention pushing a fix for one of three issues, so I'm not sure you've covered the rest as well. Also, it will be difficult to resolve that "just pushed" reference later.

@dimvar
Copy link

dimvar commented Apr 16, 2012

9679307 fixes issue 2. Issue 3 was not an issue. I didn't fix issue 1 b/c Windows issues aren't a priority for drjs at the moment.

@clausreinke
Copy link
Author

I work on Windows. I submitted sufficient fixes/workarounds to get drjs working on Windows some 7 months ago.

The project has been unresponsive for all this time, so I had to keep my own version of drjs around (which works on windows, with recent node, and includes support for my lexical-scope-aware tags extension).

I was hoping you'd finally bring drjs back from the dead (in particular, I am hoping for a fix to #5, and a merge of my lexically-scoped tags extensions, so that I can start thinking about combining the two). However, if you insist on breaking drjs on Windows, in spite of having patches that help to unbreak it, I will still not be able to use your code, nor to contribute to it.

Please reopen this ticket. Item 1 hasn't been fixed, item 3 (which is the same as #32) hasn't been addressed.

@dimvar
Copy link

dimvar commented Apr 16, 2012

Regarding Windows support:

There isn't anyone working full-time on drjs.

Personally, when I have a little time to look at the open issues, I prefer to spend my time on analysis-related bugs. I think that fixing Windows issues is not the best use of my time, given that there isn't enough demand. So, I know with certainty that I won't be working on Windows issues anytime soon.

If you find sm at Mozilla who wants to fix them, that's great. As far as I know, Patrick and Dave are also busy with other things at the moment and aren't working on drjs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants