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

use the path given in --display instead of Common.find_file result #4943

Closed
wants to merge 3 commits into from

Conversation

ousado
Copy link
Contributor

@ousado ousado commented Mar 15, 2016

This change makes sure completion uses the file given in --display instead of the Common.find_file result, which might return a different path from Common.file_lookup_cache instead.

It allows authors of haxe plugins for editors/IDEs to use a temporary directory and file for the intermediate changes to a file during typing. Please see #4651 for a number of plugin authors who weighed in and called for a solution for the general problem of having to save the original file on almost every keystroke, and this comment #4651 (comment) describing the exact problem this simple and unintrusive change solves.

An example for the use of a temporary directory can be found in the vscode-haxe plugin when using the "haxe.haxeTmpDirectory": "" setting. This way, changes to the original project file are only made when explicitly saving it, which arguably also is the expected behavior for most editors and IDEs, and hence consistent with the principle of least surprise.

Other advantages include:

  • reduced disk io in the compiler (as opposed to checking the classpaths)
  • reduced disk io for plugins when using a temporary file that is stored in memory, like under /dev/shm on Linux or files created with FILE_ATTRIBUTE_TEMPORARY on windows
  • third party tools that react to file changes aren't triggered on every keystroke (which is what originally brought me here)

@Simn Simn added this to the 3.3.0-rc1 milestone Mar 15, 2016
@Simn Simn added the feature-ide IDE / Editor support label Mar 15, 2016
@Simn
Copy link
Member

Simn commented Mar 15, 2016

This makes a lot of sense to me (partially because it was my idea). @ncannasse: Could you verify?

@ousado
Copy link
Contributor Author

ousado commented Mar 15, 2016

Indeed, sorry for omitting that: @Simn suggested this implementation and comments and work by @nadako, @jeremyfa and @pleclech made implementation and testing very easy. Thanks!

@Simn Simn assigned Simn and unassigned ncannasse Mar 15, 2016
@Simn
Copy link
Member

Simn commented Mar 15, 2016

This doesn't quite work yet because it matches too much.

@ousado
Copy link
Contributor Author

ousado commented Mar 18, 2016

closing and moving to a branch for now

@ousado ousado closed this Mar 18, 2016
@ousado
Copy link
Contributor Author

ousado commented Mar 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-ide IDE / Editor support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants