Improve zigup default and zigup list output
#44
Conversation
Default command now also points to the actual directory if default compiler is master. List command now also marks which one is the master also also marks the default compiler with ``(default)``.
|
I think all changes have been resolved. |
iddev5
force-pushed
the
better-default
branch
from
6bb1293 to
8ed3112
Compare
zigup.zig
Outdated
| const install_dir_string = try getInstallDir(allocator, .{ .create = false }); | ||
| defer allocator.free(install_dir_string); | ||
|
|
||
| var install_dir = std.fs.openDirAbsolute(install_dir_string, .{ .iterate = true }) catch |e| switch (e) { | ||
| error.FileNotFound => return, | ||
| else => return e, | ||
| }; | ||
| defer install_dir.close(); | ||
|
|
||
| const master_dir = try getMasterDir(allocator, &install_dir); | ||
| defer if (master_dir) |master| allocator.free(master); | ||
|
|
||
| if (master_dir) |master| { | ||
| if (mem.eql(u8, master, default_compiler)) { | ||
| try stdout.print("master -> ", .{}); | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not so sure about this one. It might be a little odd for zigup to call a version "master" when it's very likely that the actual version of master has moved on to a new version. Although it's true that the local "master" will link to a specific version, this could cause confusion to the user as to whether zigup actually tries to download the new version of master and verifies that master has not moved on yet.
There was a problem hiding this comment.
The behavior is actually in line with how currently zigup default master works and is meant to work.
The goal of default command as discussed in #33 was that default master command should not be given network access, and it should always work locally. Hence, I think, simple default command should respect it too.
What you're actually saying can be accomplished by zigup fetch-index | jq .master.version, and maybe it can be integrated in a cli flag or fetch-index command, but it feels like a different issue
There was a problem hiding this comment.
I realized zigup default master is actually a special case because zigup master does use network access and having zigup default master use network access as well is just redundant. In this case with list we're including details about each compiler so it's not clear whether zigup would be trying to also get an update on what version master is actually on. It's a command that provides "information" and zigup could be doing either one.
There was a problem hiding this comment.
Maybe we could do something like (master as of DATE)?
There was a problem hiding this comment.
That's a fair solution, though a bit complex one. The details need to be discussed.
- how will it determine the DATE? Is it the mtime of the folder or would the time be stored in a file everytime master is changed? EDIT: fetching also gives a formatted date of upload of master, it can be used with the second solution
- time rendering is not present in stdlib right now, additional dependencies can be used or a temporary solution be made for it
There was a problem hiding this comment.
These are good questions. I think the ideal solution is we would track whenever the master version is fetched. That being said, it might be more effort than it's worth. This is sort of a situation where I'm not sure what the right/best solution is. When this occurs I tend to wait before making any changes. I suggest we separate this change from the changes you made to "list" and come back to it.
There was a problem hiding this comment.
Separate zigup list or zigup default?
On the other hand, since there has been no certainty, it would be better to keep this PR on hold until something is decided.
iddev5
force-pushed
the
better-default
branch
from
8ed3112 to
9b93832
Compare
Commit message:
Few doubts:
This PR till now only rebases and makes a small edit since the issue was created.
Closes #35