parse package-spec from CLI correctly (#1895)

* parse package-spec from CLI correctly

fixes #861

* copy separately for `UpdateRequest.name`

fix GItHub URL reporting in results

update tests to reflect latest code changes

* better fix for GitHub URL display issue

2 files changed, 68 insertions, 79 deletions

diff --git a/src/install/install.zig b/src/install/install.zig
index 940502ce8..fb955bc70 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -5269,85 +5269,77 @@ pub const PackageManager = struct {
             // add
             // remove
             outer: for (positionals) |positional| {
-                var request = UpdateRequest{
-                    .name = positional,
-                };
-                var unscoped_name = positional;
-                request.name = unscoped_name;
-
-                // request.name = "@package..." => unscoped_name = "package..."
-                if (unscoped_name.len > 0 and unscoped_name[0] == '@') {
-                    unscoped_name = unscoped_name[1..];
-                }
-
-                // if there is a semver in package name...
-                if (std.mem.indexOfScalar(u8, unscoped_name, '@')) |i| {
-                    // unscoped_name = "package@1.0.0" => request.name = "package"
-                    request.name = unscoped_name[0..i];
-
-                    // if package was scoped, put "@" back in request.name
-                    if (unscoped_name.ptr != positional.ptr) {
-                        request.name = positional[0 .. i + 1];
-                    }
-
-                    // unscoped_name = "package@1.0.0" => request.version_buf = "1.0.0"
-                    if (unscoped_name.len > i + 1) request.version_buf = unscoped_name[i + 1 ..];
+                var value = std.mem.trim(u8, positional, " \n\r\t");
+                switch (op) {
+                    .link, .unlink => if (!strings.hasPrefixComptime(value, "link:")) {
+                        value = std.fmt.allocPrint(allocator, "link:{s}", .{value}) catch unreachable;
+                    },
+                    else => {},
                 }
 
-                if (strings.hasPrefix("http://", request.name) or
-                    strings.hasPrefix("https://", request.name))
-                {
-                    if (Output.isEmojiEnabled()) {
-                        Output.prettyErrorln("<r>😢 <red>error<r><d>:<r> bun {s} http://url is not implemented yet.", .{
-                            @tagName(op),
-                        });
-                    } else {
-                        Output.prettyErrorln("<r><red>error<r><d>:<r> bun {s} http://url is not implemented yet.", .{
-                            @tagName(op),
-                        });
-                    }
-
+                const placeholder = String.from("@@@");
+                var version = Dependency.parseWithOptionalTag(
+                    allocator,
+                    placeholder,
+                    value,
+                    null,
+                    &SlicedString.init(value, value),
+                    log,
+                ) orelse switch (op) {
+                    .link, .unlink => null,
+                    else => brk: {
+                        value = std.fmt.allocPrint(allocator, "npm:{s}", .{value}) catch unreachable;
+                        break :brk Dependency.parseWithOptionalTag(
+                            allocator,
+                            placeholder,
+                            value,
+                            null,
+                            &SlicedString.init(value, value),
+                            log,
+                        );
+                    },
+                } orelse {
+                    Output.prettyErrorln("<r><red>error<r><d>:<r> unrecognised dependency format: {s}", .{
+                        positional,
+                    });
                     Global.exit(1);
+                };
+                switch (version.tag) {
+                    .dist_tag => if (version.value.dist_tag.name.eql(placeholder, value, value)) {
+                        value = std.fmt.allocPrint(allocator, "npm:{s}", .{value}) catch unreachable;
+                        version = Dependency.parseWithOptionalTag(
+                            allocator,
+                            placeholder,
+                            value,
+                            null,
+                            &SlicedString.init(value, value),
+                            log,
+                        ) orelse {
+                            Output.prettyErrorln("<r><red>error<r><d>:<r> unrecognised dependency format: {s}", .{
+                                positional,
+                            });
+                            Global.exit(1);
+                        };
+                    },
+                    else => {},
                 }
 
-                request.name = std.mem.trim(u8, request.name, "\n\r\t");
-                if (request.name.len == 0) continue;
-
+                var request = UpdateRequest{
+                    .name = allocator.dupe(u8, switch (version.tag) {
+                        .dist_tag => version.value.dist_tag.name,
+                        .github => version.value.github.repo,
+                        .npm => version.value.npm.name,
+                        .symlink => version.value.symlink,
+                        else => version.literal,
+                    }.slice(value)) catch unreachable,
+                    .version = version,
+                    .version_buf = value,
+                };
                 request.name_hash = String.Builder.stringHash(request.name);
+
                 for (update_requests.constSlice()) |*prev| {
                     if (prev.name_hash == request.name_hash and request.name.len == prev.name.len) continue :outer;
                 }
-
-                request.version_buf = std.mem.trim(u8, request.version_buf, "\n\r\t");
-
-                // https://github.com/npm/npm-package-arg/blob/fbaf2fd0b72a0f38e7c24260fd4504f4724c9466/npa.js#L330
-                if (strings.hasPrefix("https://", request.version_buf) or
-                    strings.hasPrefix("http://", request.version_buf))
-                {
-                    if (Output.isEmojiEnabled()) {
-                        Output.prettyErrorln("<r>😢 <red>error<r><d>:<r> bun {s} http://url is not implemented yet.", .{
-                            @tagName(op),
-                        });
-                    } else {
-                        Output.prettyErrorln("<r><red>error<r><d>:<r> bun {s} http://url is not implemented yet.", .{
-                            @tagName(op),
-                        });
-                    }
-
-                    Global.exit(1);
-                }
-
-                if ((op == .link or op == .unlink) and !strings.hasPrefixComptime(request.version_buf, "link:")) {
-                    request.version_buf = std.fmt.allocPrint(allocator, "link:{s}", .{request.name}) catch unreachable;
-                }
-
-                if (request.version_buf.len == 0) {
-                    request.missing_version = true;
-                } else {
-                    const sliced = SlicedString.init(request.version_buf, request.version_buf);
-                    request.version = Dependency.parse(allocator, String.init(request.name, request.name), request.version_buf, &sliced, log) orelse Dependency.Version{};
-                }
-
                 update_requests.append(request) catch break;
             }
 
@@ -5881,14 +5873,7 @@ pub const PackageManager = struct {
             const buf = this.lockfile.buffers.string_bytes.items;
 
             const destination_dir_subpath: [:0]u8 = brk: {
-                var alias = name;
-                if (this.lockfile.alias_map.get(package_id)) |str| {
-                    alias = str.slice(buf);
-                    std.mem.copy(u8, &this.destination_dir_subpath_buf, alias);
-                    this.destination_dir_subpath_buf[alias.len] = 0;
-                    break :brk this.destination_dir_subpath_buf[0..alias.len :0];
-                }
-
+                const alias = if (this.lockfile.alias_map.get(package_id)) |str| str.slice(buf) else name;
                 std.mem.copy(u8, &this.destination_dir_subpath_buf, alias);
                 this.destination_dir_subpath_buf[alias.len] = 0;
                 break :brk this.destination_dir_subpath_buf[0..alias.len :0];
diff --git a/src/install/repository.zig b/src/install/repository.zig
index e4f775b47..d1d494059 100644
--- a/src/install/repository.zig
+++ b/src/install/repository.zig
@@ -72,7 +72,11 @@ pub const Repository = extern struct {
 
             if (!formatter.repository.resolved.isEmpty()) {
                 try writer.writeAll("#");
-                try writer.writeAll(formatter.repository.resolved.slice(formatter.buf));
+                var resolved = formatter.repository.resolved.slice(formatter.buf);
+                if (std.mem.lastIndexOfScalar(u8, resolved, '-')) |i| {
+                    resolved = resolved[i + 1 ..];
+                }
+                try writer.writeAll(resolved);
             } else if (!formatter.repository.committish.isEmpty()) {
                 try writer.writeAll("#");
                 try writer.writeAll(formatter.repository.committish.slice(formatter.buf));