Skip to content

Change regex of abbr add to allow non-letters-only abbrs #2996

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.

Sign up for GitHub

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

Merged
faho merged 1 commit into from
May 6, 2016
Merged

Change regex of abbr add to allow non-letters-only abbrs #2996

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion share/functions/abbr.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ function abbr --description "Manage abbreviations"
set key $kv[1]
set value $kv[2]
# ensure the key contains at least one non-space character
if not string match -qr "\w" -- $key
if not string match -qr "[^\s]" -- $key
Copy link
Copy Markdown
Contributor

@krader1961 krader1961 May 6, 2016

Choose a reason for hiding this comment

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

Aren't both of these incorrect? Shouldn't the key be a contiguous sequence of non-space characters? This allows "abc def" as a valid abbreviation. But if you actually execute abbr -a "abc def" "wtf" as soon as you type abc[space] it expands to def wtf. It seems to me the correct regex is '^\S+$'.

Copy link
Copy Markdown
Contributor

@krader1961 krader1961 May 6, 2016

Choose a reason for hiding this comment

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

Okay, the regex is technically wrong but it doesn't matter because of how $mode_arg is constructed and the behavior of __fish_abbr_split. They collude to turn abbr -a "abc def" "wtf" into the equivalent of abbr -a abc "def wtf". Which is truly surprising behavior and clearly not what the user intended.

Copy link
Copy Markdown
Contributor Author

@etu etu May 7, 2016

Choose a reason for hiding this comment

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

Well, sure, technically you're right. But with this change it's not a regression any more. So it's good to go for 2.3.0.

printf ( _ "%s: abbreviation must have a non-empty key\n" ) abbr >&2
return 1
end
Expand Down