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

SublimeHaskell with hsdev #132

Merged
merged 350 commits into from
Feb 18, 2016
Merged

SublimeHaskell with hsdev #132

merged 350 commits into from
Feb 18, 2016

Conversation

mvoidex
Copy link
Member

@mvoidex mvoidex commented Feb 1, 2014

I've just updated hsdev and SublimeHaskell-hsdev. Now it works in Ubuntu & Windows.
There are also new way to scan cabal, which now takes only 3-4 (not 40-100) seconds to completely rescan all modules.
And some new commands:

  • Find declarations - shows all declarations in cabal + sources for search string
  • Insert import for symbol - inserts import for not imported symbol
  • Clear imports - minimize import lists

I'll be grateful for any suggestions and testing

@@ -1,5 +1,6 @@
[
{ "keys": ["ctrl+shift+r"], "context": [ {"key" : "is_haskell_source" } ], "command": "sublime_haskell_go_to_declaration" },
{ "keys": ["ctrl+m", "ctrl+i"], "context": [ {"key": "is_haskell_source" } ], "command": "sublime_haskell_insert_import" },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People won't like us for this one, since Ctrl+M is Sublime's default for "jump to matching bracket" (very useful).

How about Ctrl+Alt+i (and of course Shift-Ctrl-P "imp...")?

@nh2
Copy link
Member

nh2 commented Feb 2, 2014

I hope I can try this out soon.

In general, how do you feel about implementing even more on the hsdev side?

I've only taken a superquick look, but as far as I understand we still do a lot of buffer changes within Sublime. On most actions like adding imports, we could save the file, let hsdev modify it, and Sublime will automatically reload it.

Pros:

  • Less Python -- from my feeling, this might make SublimeHaskell easier to maintain
  • Easier code sharing with other editors (especially all the cursor positioning could be saved here)

Cons:

  • We actually need to save the buffer (might get around this)
  • Sometimes the reload on external change resets the cursor position to the beginning of the file
  • Gives less real editor feeling

We might actually get nicely around the reload issue by letting hsdev perform the changes in-memory and send over a diff, which Sublime could run on the buffer.

What do you think?

@mvoidex
Copy link
Member Author

mvoidex commented Feb 8, 2014

Good idea. I think, I can implement both ways for test

We actually need to save the buffer (might get around this)

hsdev allows adding data with add command, so this is valid:

PS> hsinspect file foo.hs | hsdev add

We can update inspected info with buffer contents and then invoke command, which can return new buffer contents.

The real problem is that buffer contents are usually in invalid state and inspecting can fail.
I'm using undefined to fill holes, but it's annoying.

ghost referenced this pull request in mvoidex/hsdev Feb 20, 2014
@ghost
Copy link

ghost commented Feb 20, 2014

The shorter inspection is really great news.
Sublime has become my editor of choice because of SublimeHaskell, and it was one of the only gripes I had with it. Nice, indeed.

Is hsdev supplanting ghc-mod?
I noticed it embedded within the new tree.

@mvoidex
Copy link
Member Author

mvoidex commented Feb 20, 2014

@yghor, no, ghc-mod is still used for check and maybe in some other cases
But it's not used in inspection

@ghost
Copy link

ghost commented Feb 20, 2014

I thought the quicker scan was great, but the new commands are exactly what was needed to avoid switching between the browser and Sublime. I'll run and track from this branch for now on.

Suggestions

  • Find declarations: how about a version of this that took results from Hayoo (Search declaration online)?
  • Contextual Move declaration to command that:
    1. Move region of declaration to argument file
    2. Insert import for symbol on new location
    3. Clear imports on previous location

Bug?

Clear imports is not removing imports on my sandboxes when there's a warning in the form of:

Warning:
    The import of 'Package.Module' is redundant
        except perhaps to import instances from (...)

Is this expected or should I file an issue?

@mvoidex
Copy link
Member Author

mvoidex commented Feb 20, 2014

Clear imports is using GHC flag -ddump-minimal-imports. GHC doesn't remove redundant imports, so I think it's not a bug.

@mvoidex
Copy link
Member Author

mvoidex commented Feb 27, 2014

@yghor, I think it's good idea to find declarations online

  • Move region of declaration to argument file — how smart must it be?
    • Change all explicit imports of this declaration?
    • Update export list?
  • Insert import for symbol on new location — what do you mean by 'new location'?
  • And what is 'previous location'?

@mvoidex
Copy link
Member Author

mvoidex commented May 12, 2014

Can we merge this branch?

@nh2
Copy link
Member

nh2 commented May 12, 2014

Do you think it's possible to do the following:

  • git rebase -i to separate the commits which deal with hsdev from the ones that don't (the hsdev branch contains a lot of general improvements)
  • Get hsdev up on TravisCI and have it run some tests for the things we use from SublimeHaskell, from both GHC 7.6 and 7.8?
  • Set enable_hsdev: false by default and at the same time heavily advertising it in the README, so that we get an automatically selected group of beta testers and can toggle it to default true in a week or two?

Cleaning up the history is probably going to be a bit of work, but I think it can save us a lot of trouble in the long run, especially when bisecting.

@mvoidex
Copy link
Member Author

mvoidex commented May 12, 2014

@nh2 , i've rebased 6 commits
hsdev is on TravisCI already

@nh2
Copy link
Member

nh2 commented May 17, 2014

hsdev is on TravisCI already

Doesn't seem to run any tests though - https://github.com/mvoidex/hsdev/blob/master/tests/Test.hs looks quite empty

i've rebased 6 commits

Nice, it cut down more than 1000 lines of diff. There still seems to be some amount of hsdev-unrelated changes in it, for example the switch from cabal-dev to cabal sandboxes, keybinding changes, .tmLanguage changes, the switch from sublime_plugin.*Command to SublimeHaskell*Command as a base class and so.

I also seem to have another problem: In the hsdev branch, cabal building on save doesn't work for me any more. I have the config

{
    "auto_run_tests": false,
    "enable_auto_build": true,
    "enable_auto_lint": false,
    "enable_ghc_mod": true,
    "enable_hdevtools": false,
    "inspect_modules": false,
    "enable_hsdev": false
}

and on master I get Cabal: Building [projectname] on save, but on hsdev nothing happens.

@rehno-lindeque
Copy link

I've been using this branch instead of master for some time now - I came here just to say I'm excited for it to be finished off! Thanks for all the great work :-)

@mvoidex
Copy link
Member Author

mvoidex commented Jul 14, 2014

@rehno-lindeque, thanks! I have not enough time now, but I'll try to finish it as soon as possible

@mvoidex
Copy link
Member Author

mvoidex commented Sep 24, 2014

@nh2 , could you test this branch a bit? I think, it is ready to merge with master

@nh2
Copy link
Member

nh2 commented Sep 24, 2014

I shall

@mvoidex
Copy link
Member Author

mvoidex commented Feb 15, 2016

@nh2, I've implemented most things I wanted to and ready to merge
Could you take a look at this?

@nh2
Copy link
Member

nh2 commented Feb 15, 2016

@mvoidex: I'm having a look. Some points:

  • On the current version I get

    Traceback (most recent call last):
      File "/home/niklas/opt/sublime_text_3/sublime_plugin.py", line 78, in reload_plugin
        m = importlib.import_module(modulename)
      File "./importlib/__init__.py", line 90, in import_module
      File "<frozen importlib._bootstrap>", line 1584, in _gcd_import
      File "<frozen importlib._bootstrap>", line 1565, in _find_and_load
      File "<frozen importlib._bootstrap>", line 1532, in _find_and_load_unlocked
      File "<frozen importlib._bootstrap>", line 584, in _check_name_wrapper
      File "<frozen importlib._bootstrap>", line 1022, in load_module
      File "<frozen importlib._bootstrap>", line 1003, in load_module
      File "<frozen importlib._bootstrap>", line 560, in module_for_loader_wrapper
      File "<frozen importlib._bootstrap>", line 868, in _load_module
      File "<frozen importlib._bootstrap>", line 313, in _call_with_frames_removed
      File "/home/niklas/.config/sublime-text-3/Packages/SublimeHaskell/repl.py", line 15, in <module>
        import SublimeREPL.sublimerepl as sublimerepl
    ImportError: No module named 'SublimeREPL'
    

    I think we should catch this.

  • Travis seems to be broken.

  • When hsdev is not in PATH, I get Sublime Haskell: hsdev ping: no pong regularly. Should this be the case? Ideally we would check on startup wether the executable is available, and if not, print a clear error message to the log (and probably avoid polling).

  • Also this happens even though I have "enable_hsdev": false. Or maybe it is because I have it? If I set it to true, then hsdev works.

  • When hsdev is not in the PATH but we have "enable_hsdev": true,, then there's a stack trace in the console. I think we should display a pop-up for it, like for ghc-mod.

  • When the inotify limit of the system is too low, I get Sublime Haskell: hsdev scan returns error: Exception, details: {'what': 'addWatch: resource exhausted (No space left on device)'} - this will automatically happen for big projects. I think we should have a popup for it so that the user is aware why it doesn't work.

  • What does hsdev currently put the inotify watches on, each file, or only directories? If we managed to do it with only directories, then the limit might fit more easily, but I don't remember if this is possible with inotify, or whether you have to watch all files individually.

  • When restarting after the above problem, I get Sublime Haskell: hsdev scan returns error: Exception, details: {'what': 'thread killed'}, not sure why that is. After some time it tries again, but it would be nice to find out why.

  • When I try Show types, (after increasing the inotify limit and restarting) I get Can't infer types on the status bar, but nothing in the console. It would be nice to have an explanation why it didn't work there.

  • If I try again, I get:

    Traceback (most recent call last):
      File "/home/niklas/opt/sublime_text_3/sublime_plugin.py", line 574, in run_
        return self.run(edit)
      File "/home/niklas/.config/sublime-text-3/Packages/SublimeHaskell/haskell_type.py", line 291, in run
        result = self.get_types(filename, int(line) if line else None, int(column) if column else None)
      File "/home/niklas/.config/sublime-text-3/Packages/SublimeHaskell/haskell_type.py", line 212, in get_types
        m = head_of(autocomplete.hsdev_client.module(file = filename))
      File "/home/niklas/.config/sublime-text-3/Packages/SublimeHaskell/sublime_haskell_common.py", line 122, in head_of
        if len(l) > 0:
    TypeError: object of type 'NoneType' has no len()
    

    and then

    Sublime Haskell: hsdev ping: no pong
    Sublime Haskell: hsdev ping: no pong
    

    It only comes back when I restart Sublime. What might be going on here?

  • Do you still test with Sublime Text 2 as well? (I haven't yet)

  • It's awesome that hsdev is now on Stackage, that makes it really easy to install. I'd even go as far and say that we should recommend that as the way to install it in the README.

I could not test much more since because of the above problem, most of my commands seem to kill hsdev - if we can find out why, I can do more testing.

In general, I'd be happy to merge this even if there are still some problems, especially since te default is off and I know you do lots of iterative improvements on this, which should be easier to do when it's merged to master.
Let me know if you see anything obvious on why it dies for me, then I can do more testing!

@mvoidex
Copy link
Member Author

mvoidex commented Feb 16, 2016

@nh2 , thanks for response and testing

On the current version I get

There are some commands for SublimeREPL plugin (ghci, ghci current file, cabal repl). We should detect if it's not installed and disable these commands.

Travis seems to be broken

I've enabled travis according to #244, there's no travis config for master and hsdev branches. I think, i should disable it

When hsdev is not in PATH, I get Sublime Haskell: hsdev ping: no pong regularly

We're already checking hsdev version and disabling hsdev_enable on improper hsdev version. We should also check if hsdev is in PATH

When the inotify limit of the system is too low
What does hsdev currently put the inotify watches on, each file, or only directories?

I'm using fsnotify's watchTree and watchDirectory for directories. As I understand, it put inotify watches on directories (https://github.com/haskell-fswatch/hfsnotify/blob/d93630b4184e23d35d8a3efa2636a46cca7c2254/src/System/FSNotify/Linux.hs#L73), but it can't watch recursively and adds watches for each subdirectory in this case.
There are also may be bug in my implementation, which causes setting extra watches.

Do you still test with Sublime Text 2 as well?

No, but I'll install it to test with.

It only comes back when I restart Sublime. What might be going on here?

Seems, that module is not in hsdev, we should check this and show error.

Let me know if you see anything obvious on why it dies for me, then I can do more testing!

Thanks for your help! You can also try to test it on .sublime-project with only one dummy cabal project

@mvoidex
Copy link
Member Author

mvoidex commented Feb 16, 2016

@nh2, I've just noticed I forgot to upload hsdev-0.1.6.2, which have very important fix

mvoidex added a commit that referenced this pull request Feb 18, 2016
@mvoidex mvoidex merged commit e1b99c4 into master Feb 18, 2016
@nh2
Copy link
Member

nh2 commented Feb 20, 2016

Excellent!

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

Successfully merging this pull request may close these issues.

9 participants