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

Conflicting package names #9

Open
tehsphinx opened this issue Feb 18, 2021 · 18 comments
Open

Conflicting package names #9

tehsphinx opened this issue Feb 18, 2021 · 18 comments
Labels
question Further information is requested

Comments

@tehsphinx
Copy link

tehsphinx commented Feb 18, 2021

As a user of the fyne framework who wants to use some widgets or features from fyne-x I will have to deal with package naming conflicts a lot. Both fyne and fyne-x modules have a package widgets and a package layout.

@andydotxyz
Copy link
Member

We have mirrored the names for clarity, and with Go import naming this should not be a problem.

import x-widgets "fyne.io/x/fyne/widgets`

What would your suggestion be?

@andydotxyz andydotxyz added the question Further information is requested label Feb 21, 2021
@tehsphinx
Copy link
Author

tehsphinx commented Feb 21, 2021

I'd probably try to avoid the same names as it will hopefully be used together a lot. It could actually be called xwidgets and xlayout.

Then autoimport would work in editors as the editor would know what I want to import. Especially if fyne/widgets is already imported, I can't type widget.SomeXWidget and I can't type xwidget.SomeXWidget. The first one doesn't work because there is another widget package imported already and SomeXWidget just doesn't exist there. The second does not work because there is no such package since I would need to set that alias first.

@andydotxyz
Copy link
Member

Then autoimport would work in editors as the editor would know what I want to import.

It works for me, the IDE indexes all packages.

I can't type widget.SomeXWidget and I can't type xwidget.SomeXWidget.

If you specify a package name then indeed it won't find elements that are not in that package. Search instead for "SomeXWidget" and it will find the correct package. In GoLand it will even import and name appropriately the secondary package.

@andydotxyz
Copy link
Member

Also for background, we were following the golang.org/x/ naming.
I.e. net is a builtin import and golang.org/x/net is where they extend the capabilities.

@fpabl0
Copy link
Member

fpabl0 commented Feb 23, 2021

net is a builtin import and golang.org/x/net is where they extend the capabilities.

That's a good example, however they don't have the same problem as here, because golang.org/x/net don't expose any type, function or constant with net namespace, it has divided in more packages that do not cause naming conflict with net.

I think @tehsphinx's proposal is good (indeed I was about to write an issue with the same title 😅).

In GoLand it will even import and name appropriately the secondary package.

Unfortunately, not many devs can get GoLand as it is not free, I think most of them (me included) are going to use VSCode.

Or I am not sure, but firebase go sdk use something that automatically put the name of the package as an alias:

import (
     firebase "firebase.google.com/go"
)

Looking at the source code, they have:

package firebase // import "firebase.google.com/go"

So, maybe fyne-x could have a similiar approach?
Something like:

package xwidget // import "fyne.io/x/fyne/widget"

I have not tested yet, but if it works as in firebase we would have automatically:

import (
     xwidget "fyne.io/x/fyne/widget"
)

Hope it is possible. What do you think? @andydotxyz

@andydotxyz
Copy link
Member

What you are d escribing @fpabl0 is a vanity import - and we already have them.
I'm not too sure how keen I am to have code in a package that is named differrently to it's import path.

With regards to VSCode it will search all imported packages, if you import the fyne-x widget path it will search that as well.
Just like GoLand if you omit the namespace it will search for any matching type/function.

@tehsphinx tehsphinx reopened this Feb 23, 2021
@tehsphinx
Copy link
Author

tehsphinx commented Feb 23, 2021

BTW: I'm using Goland and it uses the horrible package name widget2 by default. I don't think that solves this in a satisfying way. At least not for me.

@andydotxyz
Copy link
Member

appending "2" is what golang.org does for some of their packages, e.g. "net/http" vs "golang.org/net/http2".

This is a subjective discussion, one which was covered when we originally created the namespace. It is not going to be solved in an issue, so I have added it to our agenda for Friday https://github.com/fyne-io/fyne/wiki/Agenda-for-weekly-calls.

@tehsphinx
Copy link
Author

tehsphinx commented Feb 23, 2021

http2 is a protocol name and therefore a completely different case.

@AlbinoGeek
Copy link
Contributor

AlbinoGeek commented Feb 26, 2021

xwidget or widgetx is my preference. Will love to discuss this on the call today :)

It's actually the fault of Golang Persistent Language Server" (go-pls) that vsCode can't work with these packages.

And yeah, HTTP v HTTP2 v HTTP3 are protocol versions sadly, not related to this.

@andydotxyz
Copy link
Member

This was discussed at length on the call today, we see that there are two potential issues

  1. discoverability of the new package
  2. the name of extended packages

The first issue cannot really be solved with naming - but @AlbinoGeek pointed out that doing go get -u fyne.io/x/fyne/... in your project will make all the packages available to go-pls or IDE specific search functions.

For item 2 we went back and forth a lot, however when we realised that adding an "x" to every duplicated package would lead to import paths like: "fyne.io/x/fyne/xstorage/xrepository" which is quite ugly.
As a project we find the consistency to be useful and we hoped that developers would as well.
We did not find a compelling argument for any particular rename, so decided to leave this as-is.

@fpabl0
Copy link
Member

fpabl0 commented Apr 21, 2021

@andydotxyz Sorry to reopen this. But I have a new idea. What if we divide the things in modules? Just like aws-sdk-go-v2 did. So under widget folder we could have a new go.mod:

module fyne.io/x/fyne/widget

go 1.16

The files inside widget folder would have this package name:

package widgetx

Then when users want widgets they only need to get them instead of the whole module:

go get fyne.io/x/fyne/widget

The best thing is that now the alias would be automatic, just write widgetx and the IDE will auto-import the package as:

import widgetx "fyne.io/x/fyne/widget"

Then there is no more name conflicts, the url would be look good as before, and users won't need to download the whole repository when they only need extension widgets.
What do you think?

@fpabl0 fpabl0 reopened this Apr 21, 2021
@andydotxyz
Copy link
Member

andydotxyz commented Apr 23, 2021

That is an intriguing option, thanks.
But isn't the folder name supposed to match the package name?
Would all IDEs pick this automatic naming up? It seems like the language does not enforce it...

@fpabl0
Copy link
Member

fpabl0 commented Apr 23, 2021

But isn't the folder name supposed to match the package name?

Yes, it is, but since we are defining a new module is not required?

Would all IDEs pick this automatic naming up? It seems like the language does not enforce it...

I am not sure, I have only tested on VS Code, if you can try it in Goland, should be great :)

@andydotxyz
Copy link
Member

Yes, it is, but since we are defining a new module is not required?

I’m not sure what you mean here. Being a new module or not doesn’t affect the package name does it?

How would this work for non-module based imports? It looks like the identifier would have to be different for the different ways it is imported?

@fpabl0
Copy link
Member

fpabl0 commented Apr 26, 2021

Being a new module or not doesn’t affect the package name does it?

It does not affect, but the linter will complain. But if it is a module, the folder name does not have to be the same as the package name.

How would this work for non-module based imports? It looks like the identifier would have to be different for the different ways it is imported?

You mean when users use the GOPATH instead of the go modules?

@andydotxyz
Copy link
Member

You mean when users use the GOPATH instead of the go modules?

Yes. because we still support 1.12

@andydotxyz
Copy link
Member

Goland complains that "widgetx" is a typo by the way - otherwise it seems to work like Visual Studio

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants