Skip to content

Commit

Permalink
Black formatting discrepancies with .pyi files. Fixes #PyDev-1224
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabioz
fabioz committed Jun 17, 2023
1 parent c21002c commit d8c2bbc
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 25 deletions.
50 changes: 37 additions & 13 deletions plugins/org.python.pydev.ast/src/org/python/pydev/ast/formatter/PyFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,12 @@ public static void formatSelection(IDocument doc, int[] regionsForSave, IPyForma
// formatter.formatSelection(doc, startLine, endLineIndex, edit, ps);
Assert.isTrue(regionsForSave != null);

String filepath = null;
if (edit != null) {
File f = edit.getEditorFile();
filepath = f != null ? FileUtils.getFileAbsolutePath(f) : null;
}

switch (formatStd.formatterStyle) {
case AUTOPEP8:
// get a copy of formatStd to avoid being overwritten by settings
Expand All @@ -788,13 +794,13 @@ public static void formatSelection(IDocument doc, int[] regionsForSave, IPyForma
// so range is used instead
formatStdNew.autopep8Parameters += " --range " + firstSelectedLine + " " + lastSelectedLine;
}
formatAll(doc, edit, true, formatStdNew, true, false);
formatAll(filepath, doc, edit, true, formatStdNew, true, false);
} catch (SyntaxErrorException e) {
}
return;
case BLACK:
try {
formatAll(doc, edit, true, formatStd, true, false);
formatAll(filepath, doc, edit, true, formatStd, true, false);
} catch (SyntaxErrorException e1) {
}
case PYDEVF:
Expand All @@ -806,7 +812,8 @@ public static void formatSelection(IDocument doc, int[] regionsForSave, IPyForma
String formattedAsStr;
try {
boolean allowChangingBlankLines = false;
formattedAsStr = formatStrAutopep8OrPyDev(edit != null ? edit.getPythonNature() : null, formatStd, true,
formattedAsStr = formatStrAutopep8OrPyDev(filepath, edit != null ? edit.getPythonNature() : null, formatStd,
true,
doc,
delimiter, allowChangingBlankLines);
formatted = new Document(formattedAsStr);
Expand Down Expand Up @@ -904,12 +911,14 @@ public static void formatSelection(IDocument doc, int[] regionsForSave, IPyForma
/**
* This method formats a string given some standard.
*
* @param filepath may be null
* @param str the string to be formatted
* @param std the standard to be used
* @return a new (formatted) string
* @throws SyntaxErrorException
*/
/*default*/public static String formatStrAutopep8OrPyDev(IPythonNature nature, IDocument doc, FormatStd std,
/*default*/public static String formatStrAutopep8OrPyDev(String filepath, IPythonNature nature, IDocument doc,
FormatStd std,
String delimiter, boolean throwSyntaxError, boolean allowChangingBlankLines, File workingDir)
throws SyntaxErrorException {
switch (std.formatterStyle) {
Expand All @@ -924,7 +933,7 @@ public static void formatSelection(IDocument doc, int[] regionsForSave, IPyForma

return formatted;
case BLACK:
formatted = BlackRunner.formatWithBlack(nature, doc, std, workingDir);
formatted = BlackRunner.formatWithBlack(filepath, nature, doc, std, workingDir);
if (formatted == null) {
formatted = doc.get();
}
Expand All @@ -945,7 +954,11 @@ public static void formatSelection(IDocument doc, int[] regionsForSave, IPyForma
}
}

public static String formatStrAutopep8OrPyDev(IPythonNature nature, FormatStd formatStd, boolean throwSyntaxError,
/**
* @param filepath may be null
*/
public static String formatStrAutopep8OrPyDev(String filepath, IPythonNature nature, FormatStd formatStd,
boolean throwSyntaxError,
IDocument doc, String delimiter, boolean allowChangingBlankLines)
throws SyntaxErrorException {
File workingDir = null;
Expand All @@ -958,17 +971,19 @@ public static String formatStrAutopep8OrPyDev(IPythonNature nature, FormatStd fo
}
}
}
return formatStrAutopep8OrPyDev(nature, formatStd, throwSyntaxError,
return formatStrAutopep8OrPyDev(filepath, nature, formatStd, throwSyntaxError,
doc, delimiter, allowChangingBlankLines, workingDir);
}

/**
* @param nature may be null (used for formatting with black).
* @param filepath may be null
*/
public static String formatStrAutopep8OrPyDev(IPythonNature nature, FormatStd formatStd, boolean throwSyntaxError,
public static String formatStrAutopep8OrPyDev(String filepath, IPythonNature nature, FormatStd formatStd,
boolean throwSyntaxError,
IDocument doc, String delimiter, boolean allowChangingBlankLines, File workingDir)
throws SyntaxErrorException {
String formatted = formatStrAutopep8OrPyDev(nature, doc, formatStd, delimiter, throwSyntaxError,
String formatted = formatStrAutopep8OrPyDev(filepath, nature, doc, formatStd, delimiter, throwSyntaxError,
allowChangingBlankLines, workingDir);
//To finish, check the end of line.
if (formatStd.addNewLineAtEndOfFile) {
Expand All @@ -987,12 +1002,13 @@ public static String formatStrAutopep8OrPyDev(IPythonNature nature, FormatStd fo
return formatted;
}

public static void formatAll(IDocument doc, IPyFormatStdProvider edit, boolean isOpenedFile, FormatStd formatStd,
public static void formatAll(String filepath, IDocument doc, IPyFormatStdProvider edit, boolean isOpenedFile,
FormatStd formatStd,
boolean throwSyntaxError, boolean allowChangingLines) throws SyntaxErrorException {
String delimiter = PySelection.getDelimiter(doc);
String formatted;
try {
formatted = formatStrAutopep8OrPyDev(edit != null ? edit.getPythonNature() : null, formatStd,
formatted = formatStrAutopep8OrPyDev(filepath, edit != null ? edit.getPythonNature() : null, formatStd,
throwSyntaxError, doc, delimiter, allowChangingLines);
} catch (MisconfigurationException e) {
Log.log(e);
Expand Down Expand Up @@ -1033,6 +1049,11 @@ public static void main(String[] args) {
formatStd.blankLinesTopLevel = 2;
formatStd.blankLinesInner = 1;

// TODO: The filepath should be sent for each message
// (needed to support .pyi files differently)
// The protocol must be changed for that to work...
String filepath = null;

if (args[0].equals("-multiple")) {

// Continuously read contents from the input using an http-like protocol.
Expand All @@ -1041,6 +1062,7 @@ public static void main(String[] args) {
// Writes Result: Ok|Result:SyntaxError\r\nContent-Length: xxx\r\nFORMATTED_CONTENTS
// Note: Contents must be given in UTF-8 and len is in bytes.
FastStringBuffer buf = new FastStringBuffer();

while (true) {
String line = readLine(System.in, buf);
if (line == null) {
Expand Down Expand Up @@ -1086,7 +1108,8 @@ public static void main(String[] args) {
boolean allowChangingLines = true;
String newDocContents = "";
try {
newDocContents = PyFormatter.formatStrAutopep8OrPyDev(null, formatStd, true, newDoc, delimiter,
newDocContents = PyFormatter.formatStrAutopep8OrPyDev(filepath, null, formatStd, true, newDoc,
delimiter,
allowChangingLines);
} catch (SyntaxErrorException e) {
// Don't format: syntax is not Ok.
Expand Down Expand Up @@ -1124,7 +1147,8 @@ public static void main(String[] args) {
boolean allowChangingLines = true;
String newDocContents = "";
try {
newDocContents = PyFormatter.formatStrAutopep8OrPyDev(null, formatStd, true, newDoc, delimiter,
newDocContents = PyFormatter.formatStrAutopep8OrPyDev(filepath, null, formatStd, true, newDoc,
delimiter,
allowChangingLines);
} catch (SyntaxErrorException e) {
// Don't format: syntax is not Ok.
Expand Down
14 changes: 12 additions & 2 deletions plugins/org.python.pydev.ast/src/org/python/pydev/core/pep8/BlackRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@

public class BlackRunner {

public static String formatWithBlack(IPythonNature nature, IDocument doc, FormatStd std, File workingDir) {
/**
* @param filepath note that it may be null
* @return
*/
public static String formatWithBlack(String filepath, IPythonNature nature, IDocument doc, FormatStd std,
File workingDir) {
try {
Process process;
String[] parseArguments = ProcessUtils.parseArguments(std.blackParameters);
Expand All @@ -44,8 +49,13 @@ public static String formatWithBlack(IPythonNature nature, IDocument doc, Format
}
}
PythonRunner pythonRunner = new PythonRunner(nature);

String[] pathArgs = filepath != null && filepath.length() > 0
? new String[] { "--stdin-filename", filepath }
: new String[0];

Tuple<Process, String> processInfo = pythonRunner.createProcessFromModuleName("black",
ArrayUtils.concatArrays(new String[] { "-" }, parseArguments),
ArrayUtils.concatArrays(new String[] { "-" }, pathArgs, parseArguments),
workingDir, new NullProgressMonitor());
process = processInfo.o1;
cmdarrayAsStr = processInfo.o2;
Expand Down
11 changes: 10 additions & 1 deletion plugins/org.python.pydev.core/src/org/python/pydev/core/IPyFormatStdProvider.java
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
package org.python.pydev.core;

import java.io.File;

import org.eclipse.core.runtime.IAdaptable;

public interface IPyFormatStdProvider extends IAdaptable {

Object /*FormatStd*/getFormatStd();
Object /*FormatStd*/ getFormatStd();

IPythonNature getPythonNature() throws MisconfigurationException;

IGrammarVersionProvider getGrammarVersionProvider();

IIndentPrefs getIndentPrefs();

/**
* The editor file is needed because different file extensions may
* have different formatting applied (i.e.: .py, .pyi).
*
* Note that it may return null (especially in tests).
*/
File getEditorFile();
}
2 changes: 1 addition & 1 deletion plugins/org.python.pydev/src/org/python/pydev/editor/StyledTextForShowingCodeFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public Tuple<String, StyleRange[]> formatAndGetStyleRanges(FormatStd formatStd,

try {
Document doc = new Document(str);
PyFormatter.formatAll(doc, null, false, formatStd, false, true);
PyFormatter.formatAll(null, doc, null, false, formatStd, false, true);
str = doc.get();
} catch (SyntaxErrorException e) {
}
Expand Down
17 changes: 16 additions & 1 deletion plugins/org.python.pydev/src/org/python/pydev/editor/actions/PyFormatAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
*/
package org.python.pydev.editor.actions;

import java.io.File;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.runtime.IPath;
import org.eclipse.jface.action.IAction;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.DocumentRewriteSession;
Expand All @@ -29,6 +32,7 @@
import org.python.pydev.editor.PyEdit;
import org.python.pydev.editor.PySelectionFromEditor;
import org.python.pydev.parser.prettyprinterv2.IFormatter;
import org.python.pydev.shared_core.io.FileUtils;

/**
* @author Fabio Zadrozny
Expand Down Expand Up @@ -150,7 +154,18 @@ public void formatAll(IDocument doc, IPyFormatStdProvider edit, IFile f, boolean
// formatter.formatAll(doc, edit);

FormatStd formatStd = (FormatStd) (edit != null ? edit.getFormatStd() : PyFormatterPreferences.getFormatStd(f));
PyFormatter.formatAll(doc, edit, isOpenedFile, formatStd, throwSyntaxError, true);
String filepath = null;
if (edit != null) {
File editorFile = edit.getEditorFile();
if (editorFile != null) {
filepath = FileUtils.getFileAbsolutePath(editorFile);
}
}
if (filepath == null && f != null) {
IPath path = f.getLocation().makeAbsolute();
filepath = path.toOSString();
}
PyFormatter.formatAll(filepath, doc, edit, isOpenedFile, formatStd, throwSyntaxError, true);

}

Expand Down
16 changes: 9 additions & 7 deletions plugins/org.python.pydev/tests/org/python/pydev/editor/actions/PyFormatStdTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ private void checkFormatResults(String s, String expected) {
//default check (defined with \n)
try {
Document doc = new Document(s);
PyFormatter.formatAll(doc, null, true, std, false, true);
PyFormatter.formatAll(null, doc, null, true, std, false, true);
String formatStr = doc.get();

if (DEBUG) {
Expand All @@ -1052,7 +1052,7 @@ private void checkFormatResults(String s, String expected) {
String expected2 = expected.replace('\n', '\r');

doc = new Document(s);
PyFormatter.formatAll(doc, null, true, std, false, true);
PyFormatter.formatAll(null, doc, null, true, std, false, true);
formatStr = doc.get();
assertEquals(expected, formatStr);

Expand All @@ -1061,11 +1061,12 @@ private void checkFormatResults(String s, String expected) {
expected = StringUtils.replaceAll(expected, "\r", "\r\n");

doc = new Document(s);
PyFormatter.formatAll(doc, null, true, std, false, true);
PyFormatter.formatAll(null, doc, null, true, std, false, true);
formatStr = doc.get();
assertEquals(expected, formatStr);

formatStr = PyFormatter.formatStrAutopep8OrPyDev(null, new Document(s2), std, "\r", false, true, null);
formatStr = PyFormatter.formatStrAutopep8OrPyDev(null, null, new Document(s2), std, "\r", false, true,
null);
if (expected2.endsWith("\r") && !formatStr.endsWith("\r")) {
expected2 = expected2.substring(0, expected2.length() - 1);
}
Expand All @@ -1075,7 +1076,8 @@ private void checkFormatResults(String s, String expected) {
String s3 = StringUtils.replaceAll(s, "\n", "\r\n");
String expected3 = StringUtils.replaceAll(expected, "\n", "\r\n");

formatStr = PyFormatter.formatStrAutopep8OrPyDev(null, new Document(s3), std, "\r\n", false, true, null);
formatStr = PyFormatter.formatStrAutopep8OrPyDev(null, null, new Document(s3), std, "\r\n", false, true,
null);
if (expected3.endsWith("\r\n") && !formatStr.endsWith("\r\n")) {
expected3 = expected3.substring(0, expected3.length() - 2);
}
Expand All @@ -1084,7 +1086,7 @@ private void checkFormatResults(String s, String expected) {
//now, same thing with different API
doc = new Document();
doc.set(s);
PyFormatter.formatAll(doc, null, true, std, false, true);
PyFormatter.formatAll(null, doc, null, true, std, false, true);
assertEquals(expected, doc.get());
} catch (SyntaxErrorException e) {
throw new RuntimeException(e);
Expand Down Expand Up @@ -1286,7 +1288,7 @@ public void testFormatError() throws Exception {
"";
final PyFormatAction pyFormatStd = new PyFormatAction();
try {
PyFormatter.formatAll(new Document(s), null, false, std, true, true);
PyFormatter.formatAll(null, new Document(s), null, false, std, true, true);
fail("Expecting exception!");
} catch (Exception e) {
}
Expand Down
7 changes: 7 additions & 0 deletions plugins/org.python.pydev/tests/org/python/pydev/editor/actions/PyOrganizeImportsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
*/
package org.python.pydev.editor.actions;

import java.io.File;

import org.eclipse.jface.text.Document;
import org.python.pydev.core.IGrammarVersionProvider;
import org.python.pydev.core.IIndentPrefs;
Expand Down Expand Up @@ -68,6 +70,11 @@ public Object getFormatStd() {
public <T> T getAdapter(Class<T> adapter) {
return null;
}

@Override
public File getEditorFile() {
return null;
}
};

/*
Expand Down

0 comments on commit d8c2bbc

Please sign in to comment.