Skip to content
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

Addressing issue #6, making assets location user-configurable via a n… #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Nos78
Copy link

@Nos78 Nos78 commented Nov 7, 2022

Added new configuration setting, xmlEditor.resourcesPath, which is accessible via the File->Settings menu within VSCode, under the extensions tab, with the title "Android XML Editor".

package.json:
Add new configuration properties for xmlEditor.resourcesPath, as a string with a default value to match the existing hard-coded location (modified to reflect the standard location of /res/ rather than /resources/).

Added a markdownDescription for this setting, which explains the default value and that this path is relative to the workspace root.

Note that when you add settings (or remove them) into an extension package.json, you need to reboot vscode to re-build the settings menu, then reboot vscode a second time to actually see it.

src/extension.ts:

  • Modified the createOrShow() function to get the configuration via vscode API, and get the resources path from the settings configuration object. If this path is not defined or empty, we fall back to the (modified) hard-coded path.
  • Replaced the hard-coded path with the new variable from the settings.

This commit addresses issue #6 , change 1. This pull request also factors in the change/pull request submitted by EduApps-CDG that addressed issue #4

…ble via a new configuration setting, xmlEditor.resourcesPath, which is accessible via the File->Settings menu within VSCode.

package.json:
Add new configuration properties for xmlEditor.resourcesPath, as a string with
a default value to match the existing hard-coded location (modified to reflect
the standard location of /res/ rather than /resources/).

Added a markdownDescription for this setting, which explains the default value
and that this path is relative to the workspace root.

Note that when you add settings (or remove them) into an extension package.json,
you need to reboot vscode to re-build the settings menu, then reboot vscode a
second time to actually see it.

src/extension.ts:
+ Modified the createOrShow() function to get the configuration via vscode API,
and get the resources path from the settings configuration object. If this path
is not defined or empty, we fall back to the (modified) hard-coded path.
+ Replaced the hard-coded path with the new variable from the settings.

This commit addresses issue Knowcode-AI#6, change 1.
…nowcode-AI#6

The /media/drawable directory does not exist when the extension is initially
installed, yet the createOrShow() function calls fs.rmdirSync recursively,
which throws an error on the non-existing drawable directory (media exists).

I've wrapped this call up with an fs.existsSync, which will perform the
existing action if the directory exists, otherwise it calls fs.mkdirSync
with the same options as the call to fs.rmdirSync (i.e, recursive: true)

The extension will now execute successfully on an initial install without the
user having to create the subdirectory tree manually.
@Nos78
Copy link
Author

Nos78 commented Nov 7, 2022

Seems that any commit I do to my fork automatically contributes to this same pull request... I wanted to do separate pull requests so that these fixes where kept separate. Hey ho, neither change is major so hopefully this won't be a problem.

With the second commit, I've fixed issue #5 and so the extension will now work properly without crashing after an initial install.

I won't bother making the /media/drawable cache location user-configurable - it'll probably complicate the issue for users who already have a problem after install. The project location, on the other hand, looks so much better as a user-configurable setting. This way you're not precluding users who have workspaces open from folders other than /app/...

Added instructions for the new assets directory configuration.
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.

2 participants