support optional `browse/` in osm url at open location [half patch]

[Klumbumbus]

Keepright uses this url schema in the popup:
​https://www.openstreetmap.org/browse/way/119890471

I tried this:

DownloadOsmIdTask.java

@@ -23 +23 @@
  */
 public class DownloadOsmIdTask extends DownloadOsmTask {
 
-    private static final String URL_ID_PATTERN = "https?://(?:www\\.)?(osm|openstreetmap)\\.org/(node|way|relation)/(\\p{Digit}+).*";
+    private static final String URL_ID_PATTERN = "https?://(?:www\\.)?(osm|openstreetmap)\\.org/(browse/)?(node|way|relation)/(\\p{Digit}+).*";
 
     @Override
     public String[] getPatterns() {

however when I compile this it does not work. Could someone please review? Thanks.

[Don-vip]

you have to make the new group as non-capturing by adding ?: before browse/:
private static final String URL_ID_PATTERN = "https?://(?:www\\.)?(osm|openstreetmap)\\.org/(?:browse/)?(node|way|relation)/(\\p{Digit}+).*";
Otherwise the group indexes are modified. See ​javadoc to understand how "groups and capturing" work.

[Klumbumbus]

Ok, so this would work too, right?

DownloadOsmIdTask.java

@@ -23 +23 @@
  */
 public class DownloadOsmIdTask extends DownloadOsmTask {
 
-    private static final String URL_ID_PATTERN = "https?://(?:www\\.)?(osm|openstreetmap)\\.org/(node|way|relation)/(\\p{Digit}+).*";
+    private static final String URL_ID_PATTERN = "https?://(?:www\\.)?(osm|openstreetmap)\\.org/(browse/)?(node|way|relation)/(\\p{Digit}+).*";
 
     @Override
     public String[] getPatterns() {
@@ -34 +34 @@
     public Future<?> loadUrl(DownloadParams settings, String url, ProgressMonitor progressMonitor) {
         final Matcher matcher = Pattern.compile(URL_ID_PATTERN).matcher(url);
         if (matcher.matches()) {
-            final OsmPrimitiveType type = OsmPrimitiveType.from(matcher.group(2));
-            final long id = Long.parseLong(matcher.group(3));
+            final OsmPrimitiveType type = OsmPrimitiveType.from(matcher.group(3));
+            final long id = Long.parseLong(matcher.group(4));
             final PrimitiveId primitiveId = new SimplePrimitiveId(id, type);
             final DownloadPrimitivesWithReferrersTask downloadTask = new DownloadPrimitivesWithReferrersTask(
                     settings.isNewLayer(), Collections.singletonList(primitiveId), true, true, null, null);

[Don-vip]

At first sight yes (did not check). But it's cleaner to create non-capturing groups when you don't need the value.

[Klumbumbus]

In 14944/josm:

fix #17544 - support optional browse/ in osm url at open location

[Klumbumbus]

OK, thanks for the help.