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

Utilized pragmaUtil Functions for kb.json file #854

Merged
merged 3 commits into from
May 6, 2019
Merged

Utilized pragmaUtil Functions for kb.json file #854

merged 3 commits into from
May 6, 2019

Conversation

njkevlani
Copy link
Contributor

Utilized pragmaUtil Functions (processBeforeUpload / processBeforeWrite) for keybindings.json file as well

Changes proposed in this pull request:

  • Regx for opensCurlyBraces is changed
  • Variable name are made more generic
  • Same functions are used for keybindings.json which were used for settings.json

Fixes: #800 #515

How Has This Been Tested?

  • Manually used for syncing my settings.
  • Testing using Launch Tests was failing before any new changes. We need to fix them.

Checklist:

  • I have read the contribution guidelines.
  • My change requires a change to the documentation and GitHub Wiki.
  • I have updated the documentation and Wiki accordingly.

@shanalikhan
Copy link
Owner

The acceptance criteria for #515 is mentioned at #515 (comment)

Can you add that configuration ?
It should use separate keybindings for mac as default , by setting it to true it will merge the keybindings.

@njkevlani
Copy link
Contributor Author

By the issue link in above comment, what I understood is that I need to implement a setting named sync.universalKeybindings.

My question is, what should be behavior of the extension when sync.universalKeybindings is set to false?

Some way, what should be behavior of the extension when sync.universalKeybindings is set to true?

@shanalikhan
Copy link
Owner

shanalikhan commented Apr 21, 2019

  1. When sync.universalKeybindings: true
  • Remove the keybindingsMac.json if existed

  • Insert the vscode keybindings.json to one and only keybindings.json

  1. When sync.universalKeybindings: false
  • Keep the current way as intended as seperating the mac and windows settings.

By default, it should be sync.universalKeybindings: false

You need to put new settings on Global Settings of Settings Sync.

@njkevlani
Copy link
Contributor Author

Hey @shanalikhan,

I've implemented the universalKeybindings setting. I currently do not have access to a macOS machine, so I could not test it on macOS. Can you or some other contributor test this setting on macOS?

@shanalikhan shanalikhan added this to the v3.3.0 milestone Apr 23, 2019
@njkevlani
Copy link
Contributor Author

@shanalikhan

Any updates?

@shanalikhan
Copy link
Owner

Can you or some other contributor test this setting on macOS?

Can you create a package of your code and upload the build in comment on #515 as there are users who are using mac and may be willing to test it.

@njkevlani
Copy link
Contributor Author

upload the build in comment on #515

Github does not support .vsix file uploads :(

@shanalikhan
Copy link
Owner

you can post dropbox or any other link to let them download.

@auxves
Copy link
Contributor

auxves commented May 4, 2019

@njkevlani You can also use Discord for unlimited file storage using attachments :)

@shanalikhan shanalikhan merged commit a387c23 into shanalikhan:v3.3.0 May 6, 2019
@njkevlani
Copy link
Contributor Author

@shanalikhan

Have updated wiki for pragmaUtils.

LMK if looks good. Do we need to add it somewhere else as well?

@shanalikhan
Copy link
Owner

shanalikhan commented May 8, 2019

Examples are fine, but you can improve that wiki page by adding more details or formatting the headings.
Because for the first time user, this page seems hard to understand at a first glance.

@shanalikhan shanalikhan mentioned this pull request May 8, 2019
@njkevlani
Copy link
Contributor Author

@shanalikhan

you can improve that wiki page by adding more details or formatting the headings

Can you explain what details / formatting?

first time user, this page seems hard to understand at a first glance.

How can we make it easy to understand?
Some options are:

  1. Make a different wiki page for keybindings.json
  2. Keep example / descriptions for settings.json in one section and keybindings.json in another section in a single wiki page

LMK what are your thoughts :)

shanalikhan pushed a commit that referenced this pull request Jun 24, 2019
* Utilized pragmaUtil Functions for kb.json file

* Typo fix

* Added universalKeybindings setting
shanalikhan pushed a commit that referenced this pull request Jul 1, 2019
* Updated to task api 2.0.0 and fixed tests

* Set "extensionKind": "ui" to support remote development #61

Fixes #870. See that issue for details

* Clean up previous commits into one

* #800 #515 #854

* Utilized pragmaUtil Functions for kb.json file

* Typo fix

* Added universalKeybindings setting

* chore(package): update @types/node to version 12.0.0 (#873)

* Cleaned up toggling commented settings (#877)

* Cleaned up toggling commented settings

* Updated to include fix by @ioprotium

* Fix warning with webpack

* Update UI

* Change height to max-height

* Add image locally and move it to the top

* Added documentation for tests in CONTRIBUTING.md (#881)

* fix(package): update fs-extra to version 8.0.0 (#882)

* chore(package): update @types/fs-extra to version 7.0.0 (#885)

* chore(package): update @types/node to version 12.0.1 (#883)

* fix(package): update fs-extra to version 8.0.1 (#884)

* Update UI to fit guidelines

* chore(package): update @types/node to version 12.0.2 (#887)

* Improve UI and functionality

* Match BG of theme and properly contrast text

* Fix issues with themes

* Fix image not showing up with light theme

* Fix issues with contrast

* Fix issue with VS Code's WebView

* Update UI and functionality

* Fix potential issue with light themes

* Fix issues with theme on modal

* Refactor

* Add new (but experimental) localizations (#898)

* chore(package): update clean-webpack-plugin to version 3.0.0 (#899)

*  #891

* Update settings page after a reset

* Improve code

* fix(clean-webpack-plugin): (#901)

clean-webpack-plugin was recently updated, and it had breaking changes.

* chore(package): update @types/node to version 12.0.3 (#897)

* #668

* fix(OSS):

enums.ts
----
Allow for setting os type with less code

environmentPath.ts
----
Find code folder and extensions folder more efficiently
Get OS type more efficiently

settings.ts
----
Ignore all state files

sync.ts
----
Adapt for changes in pluginService

pluginService.ts
----
Use vscode command to install extensions
Clean up code and simplify names

* Install extensions in order

* Comment legacy code instead of deleting it

* Fix extension install

* Fix issue with extensions always being deleted

* Get code path more efficiently

* Make sure to escape spaces in cli path

* Fix issues with portable builds

* Support Windows and certain editions

* Improve method for finding extension folder

* Start using API for installation

* Prepare for new API command

* Get ready for new API command

* Fix issue with installation

* Commit requested changes

* #668

* Format pluginService.ts (#906)

* #668

* Throw error

* Try to fix issues with getting missing

* chore(package): update @types/node to version 12.0.7 (#904)

* chore(package): update @types/node to version 12.0.8 (#908)

* #668

* #668

* #668

* #839

* Support multiple instances

* Completely switch to state

* Update to work properly

* Move setting classes to their own models

* Update service names

* Only auto upload using focused window

* Resolve merge conflicts

* #886

* Resolve locale properly

* Clean constructor

* Use state instead of passing in argument

* Update to be compatible with v3.3.0 changes

* Fix readme

* Fix package.json

* Add new settings

* Increase contrast

* Update

package.nls.*.json
----
Add localizations for setting names and placeholders.

github.oauth.service.ts
----
Handle errors in a more user-facing way
Support GitHub enterprise
Show message to user after success

webview.service.ts
----
Use new localizations for names and placeholders

* Bug fix and upload images

* Add support for commits to master (release notes)

* Fix issue

* Fix another issue with release notes

* Update release note template

* Update release notes

* Don't open gist selection if there aren't any gists

* Allow user to view gist

* Revert accidental change

* Support public gist

* Add css, js, and fonts to repo

* Improve font injection

* Revert accidental change
@pcdevil
Copy link

pcdevil commented Jul 18, 2019

Hey @shanalikhan,
I've noticed you merged this feature to the 3.3.0 which is not existing anymore and as I can see it's not merged back to master either. Because of this the feature is not exist in 3.4.0 version.
Was that by mistake or is there a reason why this not introduced in the current version/master?

@shanalikhan
Copy link
Owner

It is available on master and v3.4.1 ( upcoming branch ) so It was merged properly and should be working.

If you are having problem. Can you open an issue with detailed reproducing steps so i can look into it or send PR :)

@pcdevil
Copy link

pcdevil commented Jul 18, 2019

Sorry, I think somehow mixed up the branches, the code I was looking for is indeed in the master branch.

I wanted to try out the new universalKeybindings functionality but didn't seem to work. I'll make more testing and come back at you if something is wrong!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants