Skip to content

feature: allow realms to have dashes in name#51

Merged
phalestrivir merged 1 commit into
mainfrom
feature/realms-include-dashes
May 18, 2026
Merged

feature: allow realms to have dashes in name#51
phalestrivir merged 1 commit into
mainfrom
feature/realms-include-dashes

Conversation

@brycentrivir
Copy link
Copy Markdown

Escape '-' as '--' to preserve literal dashes in realm names.

Copy link
Copy Markdown

@phalestrivir phalestrivir left a comment

Choose a reason for hiding this comment

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

I suggested some improvements to your code to help simplify it. The only other things you need to do is rebase with main (since there were some new changes added there) and then run npm run lint:fix after you make all the changes. Also double-check the tests are still passing in the lib/CLI, and then you should be good to go ahead and create the PR against Rockcarver.

Comment thread src/utils/ForgeRockUtils.ts
Comment thread src/utils/ForgeRockUtils.ts Outdated
Comment on lines +143 to +157
if (realm === 'root') {
return '/';
}
return realm.replace('root-', '/').replaceAll('-', '/');

let stripped = realm.replace(/^root-/, '');

stripped = stripped.replace(/--/g, '__DASH__');

const parts = stripped.split(/-(?!_)/);

const decoded = parts.map((p) =>
p.replace(/__DASH__/g, '-')
);

return '/' + decoded.join('/');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't like the idea of replacing the double dashes with a temporary string, it seems like a complicated solution to something that should be more simple. Here's a more simpler way to do this that I came up with using ChatGPT with help for the regex:

return realm === 'root' ? '/' : realm.replace(/^root-|--|-/g, m => m === '--' ? '-' : '/');

@brycentrivir brycentrivir force-pushed the feature/realms-include-dashes branch from 3390f7b to a64ea6e Compare May 4, 2026 15:37
@phalestrivir phalestrivir merged commit 9f69e25 into main May 18, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants