-
Notifications
You must be signed in to change notification settings - Fork 185
add user settings #94
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
Conversation
|
|
||
| public static String CONFIG_LOG_LEVEL = "vscode.java.configLogLevel"; | ||
|
|
||
| public static String UPDATE_USER_SETTINGS = "vscode.java.updateUserSettings"; |
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.
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; |
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.
Can I understand why 100 -> 200?
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.
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, |
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.
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.
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.
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); |
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.
Same here, do we really need to apply settings during processing a request?
…va-debug into andy_user_settings2
| Object valueObj = map.get(keyStr); | ||
| try { | ||
| switch (keyStr) { | ||
| case "show_hex": |
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.
Not consistent with the key on vscode user settings.
|
|
||
| public final class UserSettings { | ||
| private static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME); | ||
| public static int maxStringLength = 0; |
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.
Not sure if it is a good idea to expose the (static) fields directly.
No description provided.