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

feat(smartAutocomplete): Go to Definition #104

Merged

Conversation

hiaux0
Copy link
Member

@hiaux0 hiaux0 commented Aug 12, 2019

Feature - Goto definition

1. From aurelia view to viewModel

${|viewModelVar}
.bind="|otherViewModelVar"

-->

class ... {
  @bindable |viewModelVar;

  |otherViewModelVar: any;
}

au-ext-goto-view-to-viewModel

2. From view to view, eg. button to button of buttons

${|button}

-->

repeat.for="|button of buttons"

au-ext-goto-view-to-view

3. Follow custom-element

<|custom-ele>
<require from="/|custom-ele">

-->

|class CustomEle {
}

custom-element-name

4. Attribute bindings

<!-- otherHtmlFile.html -->
<require from="/custom-ele">
<custom-ele
  |bind-me-now.bind="..."
></custom-ele>

-->

class CustomEle {
  @bindable |bindMeNow;
}

attribute-binding

Motivation

Previously, if I want to find the related class variable in the view, I had to copy paste the value and search in the viewModel.
Similarly button of buttons.

Known issues

  • if different viewModels have same @bindable variable names, definition leads to 'random' viewModel
  • 4. limited to .bind
    Should include .call, .two-way, ...

Todo

  • resolve conflict
  • cursor always at the end of line
  • provied definitions for my-bindable in
<custom-element
  my-bindable.bind="someValue"
>
</custom-element>

@hiaux0 hiaux0 changed the title Go to Definition [SmartAutocomplete] feat(smartAutocomplete): Go to Definition Aug 13, 2019
@EisenbergEffect
Copy link
Contributor

Awesome work @hiaux0 ! I'm sure @eriklieben is very happy to have such a great collaborator 😄

@hiaux0
Copy link
Member Author

hiaux0 commented Aug 13, 2019

Thank you @EisenbergEffect! I'm excited to contribute to Aurelia in some way 👨‍💻

@hiaux0 hiaux0 force-pushed the feat/smartAutocomplete-gotodefinition branch from 96e38b1 to 5e73d1a Compare August 16, 2019 22:35
@hiaux0 hiaux0 force-pushed the feat/smartAutocomplete-gotodefinition branch from 5e73d1a to 98c46e7 Compare August 23, 2019 18:57
* 2.
* Here, I assume, that .binding will only be true for valid attr. bindings
*/
if (attr.binding === 'bind') {
Copy link
Member Author

@hiaux0 hiaux0 Aug 23, 2019

Choose a reason for hiding this comment

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

  1. limited to .bind
    Should include .call, .two-way, ...

Should add an array with all possible bindings here

Edit: done 9f0dfe1

let bindings = [];
const aureliaParser = new Parser();
const interpolationRegex = /\$\{(.*)\}/g;
var match;
while (match = interpolationRegex.exec(fileContent)) {
bindings.push({
value: match[0],
bindingData: aureliaParser.parse(match[1])
// bindingData: aureliaParser.parse(match[1]) // some files got parsing errors
Copy link
Member Author

Choose a reason for hiding this comment

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

This line causes some issues in files with nested string interpolations
(compare #101)

So for the sake of 'goto' feature, I commented that out.
Could uncomment and have few files not work, but I'd rather have goto in 99% of my files

connection.onRequest('aurelia-view-information', async (filePath: string) => {
let fileProcessor = new ProcessFiles();
await fileProcessor.processPath();
aureliaApplication.components = fileProcessor.components;
Copy link
Member Author

Choose a reason for hiding this comment

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

Additionally, to processing files on extension startup.
We need to update aureliaApplication.components for new 'gotos'.

This is the fastest way I could provide updates

@hiaux0
Copy link
Member Author

hiaux0 commented Sep 13, 2019

@eriklieben There is one todo left, which I am currently not able to resolve

if different viewModels have same @bindable variable names, definition leads to 'random' viewModel

The problem with that is, we only get the 'targetWord', eg. twin-brother, but here, you can see two components having the same bindable.
It would be ideal to provide the small modal, which vscode gives on mulitple defs, but that is the point I cannot do.


The return here should accept an array as well, but it does not work.

image

All in all, I feel good about the current state.
Have been already using it locally for weeks now.
(Kind of a workaround for the todo would be, to click the tag name instead of the attribute itself.
You would be presented the correct file, but not the correct variable def)

@eriklieben
Copy link
Member

eriklieben commented Sep 17, 2019

awesome!

Sorry for the late response, my todo list isn't in the state I want it in :-). I will try to merge everything this Sunday and check if we can also get it released to the marketplace because it looks like that isn't happening automatically anymore and your work should also be pushed to the marketplace. Sorry for the inconvenience.

Request textDocument/definition failed.
  Message: Unhandled method textDocument/definition
  Code: -32601
@hiaux0 hiaux0 self-assigned this Oct 10, 2019
@bigopon
Copy link
Member

bigopon commented Oct 18, 2019

The problem with that is, we only get the 'targetWord', eg. twin-brother, but here, you can see two components having the same bindable.

Is it unable to read the tagname?

@hiaux0
Copy link
Member Author

hiaux0 commented Oct 19, 2019

Is it unable to read the tagname?

Yes, basically.
But that's just a limitation of the current implementation.
We parse tagnames somehwhere else in the code already, so would be just a matter of adding read-tagname-logic

@jboysen
Copy link

jboysen commented Nov 4, 2019

Is it possible to beta-test this feature? We are looking very much forward to this :)

@EisenbergEffect
Copy link
Contributor

@eriklieben What is our path to getting this in and getting it released?

@eriklieben eriklieben merged commit f49cea9 into aurelia:master Nov 5, 2019
@eriklieben
Copy link
Member

eriklieben commented Nov 5, 2019

Thanks for the reminder, sorry this was outside of my sight.

Merged it, will only need to get it released to the marketplace, but that has some issues. The token is expired or there is something with my installation of vsce.

If you want to test it locally, I think you can also do this:

git clone repo
npm install
npm i vsce -g

then uninstall the extension from vscode, and use the following command to create a package:

vsce package

(that will build the solution as well if I am correct, otherwise do npm run build as well.)

That will create a vsix file, which you can install locally:
open vscode, go to the extensions tab, and click the ... on the right top, and then "install from VSIX".

(double click on the file won't work)

oh and you need to make sure that this is in your settings.json

    "aurelia.featureToggles": {
        "smartAutocomplete": true
    },

not sure if that will be there by default or if you need to add it yourself. Once we published the extension to the marketplace, you should only need to update and have the above setting set/ then creating the package yourself isn't required, etc.

@EisenbergEffect
Copy link
Contributor

@eriklieben Let me know if you need anything from me to help you get this publishing again. I recall I had to refresh the token in the past. Just let me know.

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.

None yet

5 participants