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

[Haxe 3.3] Adds Support of -D display-stdin. closes #1311 #1318

Merged
merged 17 commits into from
Oct 11, 2016

Conversation

SlavaRa
Copy link

@SlavaRa SlavaRa commented Sep 7, 2016

No description provided.

@Neverbirth
Copy link
Contributor

Hmmm, what about multiple unsaved files at the same time? It seems it only gets the content of the current file are the rest are ignored. Am I right? Can we pass the contents of multiple files or only the one you want to get completion from? if it's the second then this seems to have severe drawbacks, doesn't it?

@nadako
Copy link

nadako commented Sep 7, 2016

It's the second. We thought about this, but decided not to overcomplicate things for now, since the main issue was that IDEs basically had to save the current file on each keystroke and this fixes it.

To do it "properly", we would have to do something like e.g. Roslyn does, when source files are synchronized in the compiler with IDE's in-memory contents using some protocol. Haxe is not ready for this right now, so the current display-stdin solution is the best we can do for the time being.

@nadako
Copy link

nadako commented Sep 7, 2016

By the way, I'm not really familiar with FD's codebase, but it looks like this PR doesn't implement passing stdin in the non-server mode, which can also be done (you just call haxe -D display-stdin --display ... and write contents to the standard input stream of the process.

@nadako
Copy link

nadako commented Sep 7, 2016

(see description here HaxeFoundation/haxe#5120)

@Neverbirth
Copy link
Contributor

Then I guess FD should still save the other files to be on the safe side?

@SlavaRa
Copy link
Author

SlavaRa commented Sep 7, 2016

I think that I could save other modified files, but not current file. @Neverbirth what you this about this?

@Neverbirth
Copy link
Contributor

It seems we wrote mostly at the same time, yeah, I'm OK with that one.

@SlavaRa
Copy link
Author

SlavaRa commented Sep 7, 2016

@Neverbirth fixed

@elsassph
Copy link
Member

elsassph commented Sep 7, 2016

Amazing stuff guys

@SlavaRa
Copy link
Author

SlavaRa commented Sep 12, 2016

@elsassph @Neverbirth @Meychi what about this PR?

HaxeComplete GetHaxeComplete(ScintillaControl sci, ASExpr expression, bool autoHide, HaxeCompilerService compilerService)
{
var sdkVersion = GetCurrentSDKVersion();
if (hxsettings.CompletionMode == HaxeCompletionModeEnum.CompletionServer && new SemVer("3.2.1").IsOlderThan(sdkVersion))
Copy link
Member

@elsassph elsassph Oct 2, 2016

Choose a reason for hiding this comment

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

Maybe do a semver greater-than-or-equal 3.3.0 if it's possible; who knows, a 3.2.2 fix release could happen.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed 7c68005


protected override string GetFileContent()
{
return Sci.Text;
Copy link
Member

Choose a reason for hiding this comment

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

Is there any possible filesize concern here? What about if you're editing a very large file?

Copy link
Author

Choose a reason for hiding this comment

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

My test file contains more than 20,000 lines of text and all ok)

SlavaRa added 3 commits October 3, 2016 22:09
Implemented SemVer::Equals(semVer)
Implemented SemVer::IsGreaterThan(semVer)
Implemented SemVer::IsGreaterThanOrEquals(semVer)
@Neverbirth Neverbirth merged commit 14a385f into fdorg:development Oct 11, 2016
@SlavaRa SlavaRa deleted the feature/1311 branch December 3, 2016 19:01
Gama11 referenced this pull request in HaxeFoundation/haxe Jul 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants