Skip to content

Conversation

@andxu
Copy link
Contributor

@andxu andxu commented Nov 6, 2017

No description provided.


public static String CONFIG_LOG_LEVEL = "vscode.java.configLogLevel";

public static String UPDATE_USER_SETTINGS = "vscode.java.updateUserSettings";
Copy link
Member

Choose a reason for hiding this comment

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

We need to not confuse the user by providing the command vscode.java.updateUserSetting. It's actually debugger settings. So suggest making it "vscode.java.updateDebuggerSettings".

public class StringObjectFormatter extends ObjectFormatter implements IValueFormatter {
public static final String MAX_STRING_LENGTH_OPTION = "max_string_length";
private static final int DEFAULT_MAX_STRING_LENGTH = 100;
private static final int DEFAULT_MAX_STRING_LENGTH = 200;
Copy link
Member

Choose a reason for hiding this comment

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

Can I understand why 100 -> 200?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the default length is the length most users saw, the 100 is too small for normal cases like connections string, path, I would advise to use 256.

if (showFullyQualifiedNames) {
options.put(SimpleTypeFormatter.QUALIFIED_CLASS_NAME_OPTION, showFullyQualifiedNames);
}
VariableUtils.applyFormatterOptions(options, evalArguments.format != null && evalArguments.format.hex,
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to apply the formatter options every time a evaluate request is processed? If the options stay inside VadiableUtils, then we can call it when a user actually changes the settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The condition for generating options is also influenced by argument, not only user settings.

if (showFullyQualifiedNames) {
options.put(SimpleTypeFormatter.QUALIFIED_CLASS_NAME_OPTION, showFullyQualifiedNames);
}
VariableUtils.applyFormatterOptions(options, varArgs.format != null && varArgs.format.hex, false);
Copy link
Member

Choose a reason for hiding this comment

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

Same here, do we really need to apply settings during processing a request?

Object valueObj = map.get(keyStr);
try {
switch (keyStr) {
case "show_hex":
Copy link
Contributor

Choose a reason for hiding this comment

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

Not consistent with the key on vscode user settings.

testforstephen
testforstephen previously approved these changes Nov 8, 2017
testforstephen
testforstephen previously approved these changes Nov 8, 2017
yaohaizh
yaohaizh previously approved these changes Nov 8, 2017

public final class UserSettings {
private static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME);
public static int maxStringLength = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it is a good idea to expose the (static) fields directly.

ansyral
ansyral previously approved these changes Nov 8, 2017
akaroml
akaroml previously approved these changes Nov 9, 2017
@andxu andxu dismissed stale reviews from akaroml and ansyral via 681f7d0 November 9, 2017 05:14
@andxu andxu dismissed stale reviews from yaohaizh and testforstephen via 681f7d0 November 9, 2017 05:14
@andxu andxu merged commit 330802a into master Nov 9, 2017
@andxu andxu deleted the andy_user_settings2 branch November 9, 2017 06:43
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.

6 participants