Details
- Reviewers
kevans jilles - Group Reviewers
portmgr - Commits
- rP460160: Extract desktop-categories, check-desktop-entries, and install-desktop-entries…
Diff Detail
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 14458 Build 14605: arc lint + arc unit
Event Timeline
| Mk/Scripts/check-desktop-entries.sh | ||
|---|---|---|
| 75 | The previous code using echo|grep matched the suffix case-insensitively. | |
| 76–78 | The expr syntax is invalid because / is interpreted as a division operator. Perhaps case ${Icon} in
/*) ;;
*)
${dp_ECHO_MSG} "${dp_PKGNAME}: Makefile warning: in desktop entry ${entry}: field 3 (Icon) should be either absolute path or icon name without extension if installed icons follow Icon Theme Specification" ;;
esac | |
| Mk/Scripts/check-desktop-entries.sh | ||
|---|---|---|
| 76 | This will work with the current implementation of expr, but it is not POSIX-compliant, since POSIX leaves it unspecified whether an initial ^ is special (the expression being implicitly anchored anyway). | |
| Mk/Scripts/check-desktop-entries.sh | ||
|---|---|---|
| 75 | True. On the other hand, I did check beforehand, there are no cases where the files are not lower cased. | |
| 76 | But we do not run in the os called POSIX, we run on the OS called FreeBSD, where the behavior is currently very well defined. | |
| Mk/Scripts/check-desktop-entries.sh | ||
|---|---|---|
| 76 | Sorry for being less constructive here. The desired behaviour can be implemented using POSIX-compliant expr as ${dp_EXPR} "x${Icon}" : x/. My point is not that we should restrict ourselves to POSIX features at all cost, but that an implementation using only POSIX features is preferable to an implementation using extensions if they are similar in other respects. In some situations, non-standard features unique to a system are more likely to change than standard features common with other systems. Given that this feature (^ in : regular expression) is implemented the same way by GNU expr, it seems not unlikely to change, though. | |
It looks like it will work.
This could be optimized a little but I think it is not worth gating the patch for it.