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

Type info is not working correctly #26

Open
rvion opened this issue Oct 23, 2015 · 13 comments
Open

Type info is not working correctly #26

rvion opened this issue Oct 23, 2015 · 13 comments
Labels

Comments

@rvion
Copy link
Collaborator

rvion commented Oct 23, 2015

I made a PR (#7) that picked the right type info to show, but the feature doesn't seem to work anymore

Now

image

Before

Exemple 1 :

capture d ecran 2015-07-29 a 13 16 35

capture d ecran 2015-07-29 a 13 16 41

capture d ecran 2015-07-29 a 13 16 47

capture d ecran 2015-07-29 a 13 17 00

Example 2:

capture d ecran 2015-07-28 a 13 40 01

capture d ecran 2015-07-28 a 13 39 45

capture d ecran 2015-07-28 a 13 39 40

capture d ecran 2015-07-28 a 13 39 32

related code was:

   def filter_enclosing(_,from_col, to_col, from_line, to_line, spans):
       return [span for span in spans if 
           (   ((span[1].get("spanFromLine")<from_line) or 
               (span[1].get("spanFromLine") == from_line and
                span[1].get("spanFromColumn") <= from_col))
           and ((span[1].get("spanToLine")>to_line) or 
               (span[1].get("spanToLine") == to_line and
                span[1].get("spanToColumn") >= to_col))
           )]
[type_string, type_span] = self.filter_enclosing(
                from_col_+1, to_col_+1,
                from_line_+1, to_line_+1,
                types)[0]
@rvion rvion changed the title Type info is not working correctl Type info is not working correctly Oct 23, 2015
@tomv564
Copy link
Collaborator

tomv564 commented Oct 23, 2015

Ah this is my bad.. I thought stack ide already considered the selection when returning results but seeing this it obviously doesn't. I'll restore the old logic shortly!

@tomv564
Copy link
Collaborator

tomv564 commented Oct 23, 2015

Should be fixed, @rvion can you let me know otherwise? Thanks!

@rvion
Copy link
Collaborator Author

rvion commented Oct 23, 2015

Wow, fast, thank you !
I'll reopen if it's not working :)

@rvion rvion closed this as completed Oct 23, 2015
@rvion rvion reopened this Oct 28, 2015
@rvion
Copy link
Collaborator Author

rvion commented Oct 28, 2015

it doesn't seem to be working

@rvion
Copy link
Collaborator Author

rvion commented Oct 28, 2015

image

image

the function filter_enclosing doens't seem to do the same thing as your modification

   def filter_enclosing(_,from_col, to_col, from_line, to_line, spans):
       return [span for span in spans if 
           (   ((span[1].get("spanFromLine")<from_line) or 
               (span[1].get("spanFromLine") == from_line and
                span[1].get("spanFromColumn") <= from_col))
           and ((span[1].get("spanToLine")>to_line) or 
               (span[1].get("spanToLine") == to_line and
                span[1].get("spanToColumn") >= to_col))
           )]

@rvion
Copy link
Collaborator Author

rvion commented Oct 28, 2015

ping @tomv564

@rvion rvion added the bug label Oct 28, 2015
@tomv564
Copy link
Collaborator

tomv564 commented Oct 29, 2015

Can you make sure your Sublime is running the latest code?

screen shot 2015-10-29 at 01 55 02

The filter_enclosing function should work the same, but could also use some tests (and it wouldn't be the first time I misunderstood it!). Instead of comparing line and column together it translates everything to Region objects and compares those.

@rvion
Copy link
Collaborator Author

rvion commented Oct 29, 2015

Yes, I'm running the latest code:

$ git log --pretty=oneline --abbrev-commit -5
10d0a28 Merge pull request #33 from tomv564/fix_reset_stackide
9c43b83 remove settings from reset call, fix restart command
f531149 Correct other-modules and offer config example
2630a43 Merge pull request #27 from tomv564/fix_selection_type_info
3f479aa fix show selection type functionality

but there is still a problem for me:

image

my guess is that we have a different version of either

  • stack (stack --version: 0.1.7.0, Git revision 14236a4f1cb349fd0a6870a2a78bbe182dd9565a x86_64) (last version as of today)
  • stack-ide (stack-ide --version: 0.1.0.0, Git revision 9d0e22e22bba2945f5d437490255c81a336d80ef (174 commits) X86_64)
  • sublime-text:
    image

@rvion
Copy link
Collaborator Author

rvion commented Oct 29, 2015

just updated everything to the latests versions, and still have the problem:

image

Are you on linux ?
I'm on mac, difference may be there:

$ uname -a 
Darwin MacBook-Air-de-rvion.local 14.5.0 Darwin Kernel Version 14.5.0: Wed Jul 29 02:26:53 PDT 2015; root:xnu-2782.40.9~1/RELEASE_X86_64 x86_64

@rvion
Copy link
Collaborator Author

rvion commented Oct 29, 2015

found the bug :)

/cc @tomv564

I have the automatic popup feature enabled:
image

And when I hover some code, the automatic type popup is wrong:
image

But when I hit Cmd+T (as you did to test the feature), it is correct
image

this is because in https://github.com/lukexi/stack-ide-sublime/blob/master/win.py#L43-L50, there is a duplicated code branch that is wrong:

        types = list(parse_exp_types(exp_types))
        if types:
            # Display the first type in a region and in the status bar
            view = self.window.active_view()
            (type, span) = types[0]
            if span:
                if Win.show_popup:
                    view.show_popup(type)

@rvion
Copy link
Collaborator Author

rvion commented Oct 29, 2015

possible fix (but keeping two different code path) would be to change

win.py

    from utility import ..., filter_enclosing

    [...]

    def highlight_type(self, exp_types):
        """
        ide-backend gives us a wealth of type info for the cursor. We only use the first,
        most specific one for now, but it gives us the types all the way out to the topmost
        expression.
        """
        type_spans = list(parse_exp_types(exp_types))
        if type_spans:
            _view = self.window.active_view()
            _type = next(filter_enclosing(_view, _view.sel()[0], type_spans), None)
            if not _type is None:
                _view.set_status("type_at_cursor", _type)
                if Win.show_popup:
                    _view.show_popup(_type)

I'm not making the PR because above code still duplicates code already in ShowHsTypeAtCursorCommand

@tomv564
Copy link
Collaborator

tomv564 commented Oct 29, 2015

Ah good find! If I have time I can leave some hints about a fix later. I'm also on OS-X with similar installed software / packages so the problem was definitely with the code :)

@tomv564
Copy link
Collaborator

tomv564 commented Oct 29, 2015

I think the suggested change looks good, if you don't have a chance to fix it then I can do up a PR later!

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

No branches or pull requests

2 participants