Formatting based on external formatter #80
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,3 +23,5 @@ hs_err_pid* | |
| *.iml | ||
| **/.gradle | ||
| build | ||
|
|
||
| src/gen | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| package org.nixos.idea.format; | ||
|
|
||
| import com.intellij.execution.ExecutionException; | ||
| import com.intellij.execution.configurations.GeneralCommandLine; | ||
| import com.intellij.execution.process.CapturingProcessAdapter; | ||
| import com.intellij.execution.process.OSProcessHandler; | ||
| import com.intellij.execution.process.ProcessEvent; | ||
| import com.intellij.formatting.service.AsyncDocumentFormattingService; | ||
| import com.intellij.formatting.service.AsyncFormattingRequest; | ||
| import com.intellij.openapi.util.NlsSafe; | ||
| import com.intellij.psi.PsiFile; | ||
| import com.intellij.util.execution.ParametersListUtil; | ||
| import org.jetbrains.annotations.NonNls; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
| import org.nixos.idea.file.NixFile; | ||
| import org.nixos.idea.lang.NixLanguage; | ||
| import org.nixos.idea.settings.NixLangSettings; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.util.EnumSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| public final class NixExternalFormatter extends AsyncDocumentFormattingService { | ||
|
|
||
| @Override | ||
| protected @NotNull String getNotificationGroupId() { | ||
| return NixLanguage.NOTIFICATION_GROUP_ID; | ||
| } | ||
|
|
||
| @Override | ||
| protected @NotNull @NlsSafe String getName() { | ||
| return "NixIDEA"; | ||
| } | ||
|
|
||
| @Override | ||
| public @NotNull Set<Feature> getFeatures() { | ||
| return EnumSet.noneOf(Feature.class); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean canFormat(@NotNull PsiFile psiFile) { | ||
| return psiFile instanceof NixFile; | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| protected @Nullable FormattingTask createFormattingTask(@NotNull AsyncFormattingRequest request) { | ||
| NixLangSettings nixSettings = NixLangSettings.getInstance(); | ||
| if (!nixSettings.isFormatEnabled()) { | ||
| return null; | ||
| } | ||
|
|
||
| var ioFile = request.getIOFile(); | ||
| if (ioFile == null) return null; | ||
|
|
||
| @NonNls | ||
| var command = nixSettings.getFormatCommand(); | ||
| List<String> argv = ParametersListUtil.parse(command, false, true); | ||
|
|
||
| var commandLine = new GeneralCommandLine(argv); | ||
|
|
||
| try { | ||
| var handler = new OSProcessHandler(commandLine.withCharset(StandardCharsets.UTF_8)); | ||
| OutputStream processInput = handler.getProcessInput(); | ||
| return new FormattingTask() { | ||
| @Override | ||
| public void run() { | ||
| handler.addProcessListener(new CapturingProcessAdapter() { | ||
|
|
||
| @Override | ||
| public void processTerminated(@NotNull ProcessEvent event) { | ||
| int exitCode = event.getExitCode(); | ||
| if (exitCode == 0) { | ||
| request.onTextReady(getOutput().getStdout()); | ||
| } else { | ||
| request.onError("NixIDEA", getOutput().getStderr()); | ||
|
There was a problem hiding this comment. question (non-blocking): Should we maybe use “Nix External Formatter” for the title? (Same in catch block.) There was a problem hiding this comment. I think it is clearer to use the plugin name as title, so it is clear who is 'to blame' for the error, but I do not have a strong opinion. I can change it to Nix External Formatter if you prefer There was a problem hiding this comment. I am fine with both. I think both have their pros and cons. 😅 |
||
| } | ||
| } | ||
| }); | ||
| handler.startNotify(); | ||
| try { | ||
| Files.copy(ioFile.toPath(), processInput); | ||
| processInput.flush(); | ||
| processInput.close(); | ||
| } catch (IOException e) { | ||
| handler.destroyProcess(); | ||
| request.onError("NixIDEA", e.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public boolean cancel() { | ||
| handler.destroyProcess(); | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isRunUnderProgress() { | ||
| return true; | ||
| } | ||
| }; | ||
| } catch (ExecutionException e) { | ||
| request.onError("NixIDEA", e.getMessage()); | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| package org.nixos.idea.settings; | ||
|
|
||
| import com.intellij.openapi.application.ApplicationManager; | ||
| import com.intellij.openapi.components.PersistentStateComponent; | ||
| import com.intellij.openapi.components.RoamingType; | ||
| import com.intellij.openapi.components.State; | ||
| import com.intellij.openapi.components.Storage; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| import java.util.ArrayDeque; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.Deque; | ||
|
|
||
| @State(name = "NixLangSettings", storages = @Storage(value = "nix-idea-lang.xml", roamingType = RoamingType.DISABLED)) | ||
| public final class NixLangSettings implements PersistentStateComponent<NixLangSettings.State> { | ||
|
There was a problem hiding this comment. polish: I would prefer if you keep this state class more specific for now. Like
There was a problem hiding this comment. for reason (1), if you add an non-external formatter, I think it would appropriate to add its settings to this class also (rather than the existing settings in the "Build" section) for reason (2), if you prefer a different file to New settings can go underneath like in the picture below: 
I renamed 'Formatter Configuration' to ' External Formatter Configuration' If you still feel like the external formatter deserves its own section, I am happy to make another one too though! Again, no strong opinions from me on this There was a problem hiding this comment. My concern was mostly about where and how to store the configuration, not the UI. The configuration of a built-in formatter would be system-independent and can be synchronized across operating systems. External formatters should not be synchronized across operating systems. To make this separation, these two cases need to be stored in separate XML files. The <application>
<component name="NixLspSettings">
<!-- LSP settings -->
</component>
<component name="NixExternalFormatterSettings">
<!-- External formatter settings -->
</component>
</application>instead of <application>
<component name="NixLspSettings">
<!-- LSP settings -->
</component>
<component name="NixLangSettings">
<!-- Other system-dependent settings -->
</component>
</application>There was a problem hiding this comment. I see, thanks for the explanation! That makes sense, I will apply your suggestion then. |
||
|
|
||
| // TODO: Use RoamingType.LOCAL with 2024.1 | ||
|
|
||
| // Documentation: | ||
| // https://plugins.jetbrains.com/docs/intellij/persisting-state-of-components.html | ||
|
|
||
| private static final int MAX_HISTORY_SIZE = 5; | ||
|
|
||
| private @NotNull State myState = new State(); | ||
|
|
||
| public static @NotNull NixLangSettings getInstance() { | ||
| return ApplicationManager.getApplication().getService(NixLangSettings.class); | ||
| } | ||
|
|
||
| public boolean isFormatEnabled() { | ||
| return myState.formatEnabled; | ||
| } | ||
|
|
||
| public void setFormatEnabled(boolean enabled) { | ||
| myState.formatEnabled = enabled; | ||
| } | ||
|
|
||
| public @NotNull String getFormatCommand() { | ||
| return myState.formatCommand; | ||
| } | ||
|
|
||
| public void setFormatCommand(@NotNull String command) { | ||
| myState.formatCommand = command; | ||
| addFormatCommandToHistory(command); | ||
| } | ||
|
|
||
| public @NotNull Collection<String> getCommandHistory() { | ||
| return Collections.unmodifiableCollection(myState.formatCommandHistory); | ||
| } | ||
|
|
||
| private void addFormatCommandToHistory(@NotNull String command) { | ||
| Deque<String> history = myState.formatCommandHistory; | ||
| history.remove(command); | ||
| history.addFirst(command); | ||
| while (history.size() > MAX_HISTORY_SIZE) { | ||
| history.removeLast(); | ||
| } | ||
| } | ||
|
|
||
| @SuppressWarnings("ClassEscapesDefinedScope") | ||
| @Override | ||
| public void loadState(@NotNull State state) { | ||
| myState = state; | ||
| } | ||
|
|
||
| @SuppressWarnings("ClassEscapesDefinedScope") | ||
| @Override | ||
| public @NotNull State getState() { | ||
| return myState; | ||
| } | ||
|
|
||
| static final class State { | ||
| public boolean formatEnabled = false; | ||
| public @NotNull String formatCommand = ""; | ||
| public Deque<String> formatCommandHistory = new ArrayDeque<>(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This directory is no-longer generated, which is why I removed it from the
.gitignore. However, it probably makes sense to keep the ignore-rule for some more time, as it might be confusing when you temporarily checkout an older commit and then have all these untracked files afterward.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah that explains why I had these files even though they were not gitignored