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

New SDK implemention, elixir-builder and so on #167

Merged
merged 18 commits into from
Jul 31, 2015
Merged

New SDK implemention, elixir-builder and so on #167

merged 18 commits into from
Jul 31, 2015

Conversation

zyuyou
Copy link
Contributor

@zyuyou zyuyou commented Jul 17, 2015

1.< new Project > is very simple, just create source-dir lib, but I will implement new one which like "mix new options"
new project
2. < Create project from existing sources > just detect the project' sdkType, maybe should detect the sourceRoot too, but mix importWizard is more powerfull.
mix-importwizard-1
mix-importwizard-2
projectdetector
3. runconfiguration which use mix tool, and I will add mix ex-unit runconfiguration too in the future because it was a difficult job.
4. elixir-builder, now we can use shift+command+F9 or command+B to compile source file or make project and module.
2015-07-16 5 03 59
2015-07-18 5 19 50
5.add some new fileTemplates and also separate new-Elixir-File from others as a new group.
2015-07-18 5 22 14
2015-07-18 5 22 36

* if "isMake": compile all files of the module and dependent module.
* else: just for compile the target affected file.
* */
private static void doBuildWithElixic(ElixirTarget target,
Copy link
Owner

Choose a reason for hiding this comment

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

  • Did you mean to name this method doBuildWithElixirc instead of doBuildWithElixic? It looks like you're missing the r at the end of Elixir.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, r missed. it should be doBuildWithElixirc

2. add `Makefile` as an easy way for local ant build
3. fix tests's path
@zyuyou
Copy link
Contributor Author

zyuyou commented Jul 24, 2015

All most of these issues mentioned in the comments above had been fixed, except the smallidea one.
and my master branch had pass the travis-ci test, but this PR has some conflicts now, considering you are fixing the parser feature, I will merge your master branch again when your work is done, ok?

@KronicDeth
Copy link
Owner

@zyuyou I just released v1.0.0, so you can now merge master into your PR branch.

@KronicDeth
Copy link
Owner

Do you have verification steps or a test plan you were using to manually test each feature? This will serve as the basis of my manual testing and the feature explanations that go in the README.

@KronicDeth KronicDeth self-assigned this Jul 27, 2015
This was referenced Jul 27, 2015
@zyuyou
Copy link
Contributor Author

zyuyou commented Jul 28, 2015

Just two tests, MixProjectImportBuilderTest for MixImportWizard and ElixirBuilderTest for ElixirBuilder.

@zyuyou
Copy link
Contributor Author

zyuyou commented Jul 29, 2015

@KronicDeth , Would you like to make ElixirFile like ErlangFile and ErlangFileImpl? so that I can get defmodule[] by using ElixirFile.getModules() like ErlangFile.getAttributes() .

if you not, in order to resolve this todo, I just got defmodule by the method showed in the screenshots, but I think that is not good, do you have any good suggestion?
2015-07-29 10 14 46

@KronicDeth
Copy link
Owner

You can resolve that TODO after I land this PR (which I'll hopefully do tonight, in 10 -12 hours). I don't want to delay landing this more than I've already done while you had to wait for me to finish v1.0.0.

@KronicDeth
Copy link
Owner

@zyuyou I merged the PR to master, but when I was updating the README with all the features you added I wasn't able to get Build > Compile or Build > Make Project to error out when there was an error in the file to be built, but mix compile does show the error.

Here's my test file:

defmodule IntellijElixir.Quoter do
  broken
  use GenServer

  def start_link(args, opts \\ []) do
    GenServer.start_link(__MODULE__, args, opts)
  end

  def handle_call(code, _from, state) do
    {:reply, Code.string_to_quoted(code), state}
  end
end

The function call broken is undefined and it is properly flagged as an error with mix compile:

== Compilation error on file lib/intellij_elixir/quoter.ex ==
** (CompileError) lib/intellij_elixir/quoter.ex:2: undefined function broken/0
    (stdlib) lists.erl:1352: :lists.mapfoldl/3
    (stdlib) erl_eval.erl:657: :erl_eval.do_apply/6

If I do Build > Compile I get "Compilation completed successfully" in the Event Log with no error message.

How did you test the Build > Compile shows errors correctly?

@KronicDeth
Copy link
Owner

My last comment may be a due to a packaging or install error. I get the proper Messages Compiles tab with errors when I run the plugin in the debugger using Plugin > Elixir run configuration.

screen shot 2015-07-30 at 8 13 13 pm

The last time something like this happened, it was due to directories not being copied in the .zip and jars correctly. I'll figure out it, no need for you to debug it.

@KronicDeth
Copy link
Owner

Confirmed it's a packaging problem. intellij-erlang's .zip contains the jps jars that are in compilerServer classpath:

screen shot 2015-07-30 at 8 50 01 pm

but intellij-elixir does not

screen shot 2015-07-30 at 8 50 11 pm

So, I just need to figure out how to get the jsp jars into the zip.

@KronicDeth
Copy link
Owner

I found a work-around: instead of using the zip made by "Build > Prepare Plugin for deployment", I do "Build > Build Artifact > intellij-elixir.zip", which includes the jps jars. I'm waiting on a response to a question in the Open API forum to see if I can go back to using "Prepare Plugin for deployment".

@KronicDeth KronicDeth merged commit 08f1848 into KronicDeth:master Jul 31, 2015
KronicDeth added a commit that referenced this pull request Jul 31, 2015
Mark as release v1.1.0 because PR is full of so many features all by
itself.
@zyuyou
Copy link
Contributor Author

zyuyou commented Jul 31, 2015

"Build > Build Artifact > intellij-elixir.zip"

Yes, I did the same. But I wonder why those .jar like junit-4.11.jar should be packaged in the final .zip

@KronicDeth
Copy link
Owner

The official answer from Jetbrains is that using Build Artifact is the correct way to do the build since "Prepare Plugin for deployment" always uses a fixed layout.

junit-4.11.jar is in the build because some of the classes in the org.elixir_lang namespace, such as org.elixir_lang.intellij_elixir.Quoter have assert* methods that wrap assert* methods from junit. Those methods are currently only used in testing and I just haven't had time to separate the testing parts of the code from the production part of the code, so junit is a dependency of intellij-elixir.zip.

@zyuyou
Copy link
Contributor Author

zyuyou commented Jul 31, 2015

❤️ thanks.

@jejacks0n
Copy link

This is great, and thanks for the work. I'm following (I think) everything that I should be doing, but keep getting the same result with my setup.

screen shot 2016-05-09 at 11 34 21 am

/usr/local/Cellar/elixir/1.2.4/bin/elixir "" -S phoenix.server
No file named 

It appears to be putting "" in the command in a way I can't circumvent. Is this to be expected, and am I using it wrong? Let me know if I should open an issue, or am doing something incorrectly.

@KronicDeth
Copy link
Owner

@jejacks0n: The "" would be the location of mix. You may have opened the project incorrectly. You need to import the project as a mix project for the location of mix to be set using The README instructions for importing a project from an external model

In the future, please open a new issue if you need help. Asking for help, even if the help is related to a PRs code, should not be done in the comments of the PR if the PR is Merged or Closed. Because this PR is Merged, it doesn't show up as something that needs to be worked on, so your comment would just disappear if I hadn't noticed the notification.

@KronicDeth KronicDeth mentioned this pull request Jul 8, 2017
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.

3 participants