Reformat code

Hi, I wanted to know which is the correct or recommended way to reformat some code.
I was using something like:

        WriteCommandAction.Simple<String> command = new WriteCommandAction.Simple<String>(project, psiFile) {
            @Override
            protected void run() throws Throwable {
                CodeStyleManager.getInstance(project).reformat(psiFile, false);
            }
        };
        command.execute();


But for some reason that code was not working fine sometimes, I don't exactly remember what was the problem (maybe it doesn't exist anymore), but now I have a final code that doesn't smell too well for me :)

public static void reformat(@NotNull final PsiElement element)
    {

        final Project project = element.getProject();
        PsiFile psiFile = element.getContainingFile().getOriginalFile();
        if(psiFile != null){
            VirtualFile virtualFile = psiFile.getVirtualFile();
            if(virtualFile != null){
                Reformatter reformatter = new Reformatter() {
                    public void run() {
                        try {
                            WriteCommandAction writeCommand = new WriteCommandAction.Simple(project, psiFile) {
                                @Override
                                protected void run() throws Throwable {
                                    CodeStyleManager.getInstance(project).reformat(element);
                                }
                            };
                            writeCommand.execute();
                            PsiDocumentManager.getInstance(project).doPostponedOperationsAndUnblockDocument(document);
                            fileDocManager.saveDocument(document);

                        } catch (IncorrectOperationException e) {
                            // LOG.error(e);
                        }

                    }
                };
                reformatter.execute(project, virtualFile);
            }
        }
    }



    private abstract static class Reformatter implements Runnable {
        PsiFile psiFile;

        FileDocumentManager fileDocManager;

        Document document;

        void execute(@NotNull Project project,
                     @NotNull VirtualFile file) {
            fileDocManager = FileDocumentManager.getInstance();
            document = fileDocManager.getDocument(file);
            if (document != null) {
                psiFile = PsiDocumentManager.getInstance(project).getPsiFile(document);
                if (psiFile != null) {
                    final Application application = ApplicationManager.getApplication();
                    final Runnable action = this;
                    application.invokeAndWait(new Runnable() {
                        @Override
                        public void run() {
                            application.runWriteAction(action);
                        }
                    }, application.getCurrentModalityState());
                }
            }
        }
    }


That code was working fine before PhpStorm8
The line:
fileDocManager.saveDocument(document);
is throwing an error now, due the multicaret, but anyway, maybe you can put a good and "safe in all situations" way to reformat the code here.

Thanks !!
12 comments
Comment actions Permalink

The correct and recommended way to reformat a file is your original implementation. If it's "not working fine sometimes", please provide details.

0
Comment actions Permalink

I don't remember exactly what was the problem.
But I think was related with some thread issue or something like that.
That's why I've added this code:

                    application.invokeAndWait(new Runnable() {
                        @Override
                        public void run() {
                            application.runWriteAction(action);
                        }
                    }, application.getCurrentModalityState());


Can you imagine any situation where the "correct" code could fail if we don't use the invokeAndWait from above?

(could the problem be related with the fact that I'm saving the document after the reformat?)

0
Comment actions Permalink

The reformat action needs to be performed from the event dispatch thread. But this also applies to all document/PSI modifications and the document saving, so it's difficult for me to imagine a situation where invokeAndWait() would be the correct thing to do.

0
Comment actions Permalink

And using only this code:

WriteCommandAction.Simple<String> command = new WriteCommandAction.Simple<String>(project, psiFile) {
             @Override
            protected void run() throws Throwable {
                CodeStyleManager.getInstance(project).reformat(psiFile, false);
            }
        };
        command.execute();


Should guarantee that is always executed from the event dispatch thread?

0
Comment actions Permalink

This code will log an error if you try to run it in a different thread. It will not automatically transfer the execution to the EDT.

0
Comment actions Permalink

So what is the correct way to reformat the code in any case?
I'm using a helper class, and I'm using the reformat method in many different places, I just want to make sure it will always work.
Is there any reliable way to do that Dmitry?
how can we check if we are in EDT and how can we force it or wait for it if we are not?

0
Comment actions Permalink

The correct way to do things is to run all of your code that performs any write operations in the event dispatch thread. invokeAndWait() will let you ensure that "it will always work", where "work" means "not log any errors", but this is not the correct way to do things. Using invokeAndWait() can also lead to deadlocks depending on the context where it is used.

0
Comment actions Permalink

OK, could you give me an example for this:
"run all of your code that performs any write operations in the event dispatch thread"
I just want to make sure it will always work.

0
Comment actions Permalink

An example for what? In most cases, plugin code is invoked from a UI element and therefore runs in the event dispatch thread; you need to do something yourself (such as using ProgressManager or executeOnPooledThread()) to move the execution to another thread. Also, all write operations already log errors when you try to perform them from another thread.

-1
Comment actions Permalink

Yes but how I've said I would like to have a Helper class, with a method "reformat" and to be able to use it anywhere without worrying abouth the thread (and change or wait for the thread if necessary) is that possible?

Maybe something like this?

if( ! EventQueue.isDispatchThread()) {
        ApplicationManager.getApplication().invokeLater(new Runnable() {
            @Override
            public void run() {


            }
        }, new Condition() {
            @Override
            public boolean value(Object o) {
                return ! EventQueue.isDispatchThread();
            }
        });
}

0
Comment actions Permalink

This particular use of the invokeLater() API is not correct. The semantics of the Condition is "don't execute the operation at all when the condition is true", which doesn't seem to match what you're trying to do here.

And the "correct and recommended" way to do things, which is what you originally asked about, is to have a clear idea of the context in which your code is running, instead of trying to write helper classes that will work in any context.

0
Comment actions Permalink

OK, Thanks Dmitry, I will try to do that, the problem is that sometimes is not so clear to me which is that "context", that was I was expecting to be able to use some Helper class with a few methods more robust to that part.

0

Please sign in to leave a comment.