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

Add support for non-directory sources #857

Merged
merged 3 commits into from Aug 9, 2019

Conversation

@tgodzik
Copy link
Collaborator

commented Aug 7, 2019

Fixes #770

Reopened the PR here. I fixed the suggestions:

  • added a file watcher which only reports on the specific file
  • if a source file item does not exist we then create the parent dir and watch it then delete it afterwards as with the source directories
  • renamed isSourceItem to isSource
@tgodzik tgodzik requested a review from marek1840 Aug 7, 2019
@marek1840

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2019

I have a stupid idea.
What about:

final class ExternalSourceWatcher(emit: FileChangedEvent => Unit) {
  private val files = mutable.Map.empty[Path, State]
  private val watcher = new Thread(() => {
    Thread.sleep(250)
    files.foreach { case (file, previousState) =>
      val currentState = ???
      if(currentState != previousState){
        val event = ???
        emit(event)
      }
    }
  })
}

that way we can avoid potentially a lot of traffic when listening for a one file in a big directory and also we don't have to handle files in a roundabout way just to make them fit in the underlying model.

@tgodzik

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 8, 2019

We would lose a lot of time to check on files even if nothing was happening, it would add more problems than just having the events from the file system I think. And using files as sources is pretty rare currently. If it proves to be a problem we can experiment on some real life projects.

@tgodzik tgodzik force-pushed the tgodzik:add-file-sources branch 3 times, most recently from 8541e89 to 444c8c6 Aug 8, 2019
@tgodzik tgodzik force-pushed the tgodzik:add-file-sources branch from 444c8c6 to 0dd3eed Aug 8, 2019
@tgodzik tgodzik requested a review from marek1840 Aug 8, 2019
@tgodzik

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 9, 2019

Currently it's failing on Windows (a couple of tests have GC issues), which we need to take a look at and the other is TreeViewSlowSuite which seems to change dependencies despite not changing them :/ Some versions of the dependencies change.

@tgodzik tgodzik merged commit afdcd37 into scalameta:master Aug 9, 2019
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
scalameta.metals Build #20190808.5 failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tgodzik tgodzik deleted the tgodzik:add-file-sources branch Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.