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

Support for context menus and localization #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

InDIOS
Copy link

@InDIOS InDIOS commented Sep 6, 2016

I added some localization messages, please review that are correct.

@lfurzewaddock
Copy link
Owner

Apologies @InDIOS, I've been away on holiday. Please give me a few days to catch up.

Copy link
Owner

@lfurzewaddock lfurzewaddock left a comment

Choose a reason for hiding this comment

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

Support for Localization is a good idea, but may need to be submitted as a separate pull request. Please await the outcome of pull request #5.

@@ -1,5 +1,7 @@
'use strict';

var fs = require('fs');
var path = require('path');
var vscode = require('vscode');
var fileService = require('./libs/fileService');
Copy link
Owner

Choose a reason for hiding this comment

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

You should be able to use the existing fileService or modify it if necessary, rather then rewrite a lot of its functionality. The 'Open File...' context menu should just add the option of the UI passing a file location to the fileService to open, in addition to the existing default behaviour of passing the current active document file location to the fileService to open activated by Command/CTRL+ALT+O.

There is another pull request #5 with similar functionality. You may wish to wait for the outcome of that to avoid duplicating efforts.

Roberto Asiel Guevara Castañeda added 2 commits September 22, 2016 00:06
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