Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

Contracts not detected #251

Merged
merged 5 commits into from
Nov 3, 2022
Merged

Contracts not detected #251

merged 5 commits into from
Nov 3, 2022

Conversation

xhulz
Copy link
Contributor

@xhulz xhulz commented Oct 31, 2022

PR description

Issue ref: #248

When more than one workspace is opened on Visual Studio Code, the Explorer View does not find the contract folder. This patch fixes this situation and displays the directory regardless of location

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if documentation updates are required.

Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

Hi @xhulz, thanks for taking care of this. Great improvement.

As a side effect, if the user has more than one Truffle workspace, the Contract Explorer shows a bunch of contracts folders, making it difficult to distinguish between each other

image

src/Constants.ts Outdated Show resolved Hide resolved
src/views/FileExplorer.ts Outdated Show resolved Hide resolved
@xhulz
Copy link
Contributor Author

xhulz commented Oct 31, 2022

Hey @acuarica,

Thank you for your review.

The issue #248 has asked to find the directory within the projects, which the developed patch solves. As for the other problems you mentioned, I will evaluate because I need to understand how this view was developed and has been working today, to see the possibility of making these corrections in the best possible way.

Thank you again

@michaeljohnbennett
Copy link
Contributor

I think this is a good step forward and the provider code I was working on was thinking about this same problem in a different way.

I think we need a more general solution to create the treeview recursively to mirror the structure of the folders/projects.

Alongside this if this was tied into a provider searching feature it would then know if folderA was truffle or another type. Interrogate the config for that folder knowing where to look for the contracts folder etc and this would then use sensible defaults and also the config when overridden.

I was working on this for the POC and it has a bit of overlap with this, I would hopefully be able to post something in the next day once I get it working a bit better.

What do you think?

Copy link
Contributor

@michaeljohnbennett michaeljohnbennett left a comment

Choose a reason for hiding this comment

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

Should we bring this feature into the POC for providers and use this style of directory traversal alongside it?

I think right now this will fix the issue with multiple contract folders but still has issues in terms of display or understanding what is where in the folders?

@xhulz
Copy link
Contributor Author

xhulz commented Nov 2, 2022

Hey @acuarica and @michaeljohnbennett

I just made 2 new tweaks:

  1. Now if you have more than one truffle project open in your vscode, the contract folders will appear like this: contract-folder-name + project name

Screenshot 2022-11-02 at 11 49 25

###########################################################################################

  1. The contract folder is being read from the truffle configuration file. You can see a preview here:

xpto-1667389095653

Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

Hi @xhulz, looking good. I've made some comments that we can either address here or in another PR, since as you mentioned earlier they are not technically related to this PR.

.vscode/settings.json Outdated Show resolved Hide resolved
src/views/FileExplorer.ts Outdated Show resolved Hide resolved
@xhulz
Copy link
Contributor Author

xhulz commented Nov 2, 2022

Hey @acuarica,

I just removed those extra lines in the settings.json file and the deduplication treatment for truffle workspaces

Thank you

@xhulz xhulz requested a review from acuarica November 2, 2022 14:17
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lgtm

One last thing about error handling. Now that the file explorer combines multiple projects into a single view, it might make sense to catch errors individually. For example, if one of the Truffle config files has an error, we might still want to display the other projects

opbox-1667399970910.mp4

@xhulz
Copy link
Contributor Author

xhulz commented Nov 2, 2022

Hey @acuarica,

Thank you, i'll do that

@michaeljohnbennett
Copy link
Contributor

yeah, this looks better 👍🏻

@michaeljohnbennett
Copy link
Contributor

I can merge this code into the POC work I'm doing and move to the AbstractWorkspace code I have refactored.

@xhulz
Copy link
Contributor Author

xhulz commented Nov 3, 2022

Hey @acuarica and @michaeljohnbennett!

I've made some tweaks and put the error message inside the file explorer box. You can see some examples below.

Thank you again

  1. No truffle configuration file:
    xpto-1667474911088

  2. Contract folder not found:
    xpto-1667474986770

  3. Truffle configuration file with incorrect format:
    optmism-1667475103020

Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lgtm

@xhulz xhulz merged commit ff6dc8f into develop Nov 3, 2022
@xhulz xhulz deleted the fix/contract-explorer branch November 3, 2022 18:02
@xhulz xhulz mentioned this pull request Nov 22, 2022
@acuarica acuarica mentioned this pull request Dec 5, 2022
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants