[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r239300359
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
> Just want to make sure this doesn't have any impact? Was this $scope.user
not used at all?
It wasn't used at all. I removed this unnecessary assignment to
`$scope.user` after a brief "how is this working at all???" moment. As that
assignment will only occur if `userService.getUser()` succeeds, anything which
depends on it would fail in cases where that failure previously resulted in a
warning, yet clearly those cases (CAS, OpenID, header auth) work just fine.
The relevant template only references `$scope.username`. Neither the
template nor the directive reference `$scope.user`.
The `$scope.user` assignment was introduced briefly via e9549fb, at which
point it should have been documented (but wasn't). The usefulness of that
assignment was removed via 06fb054. Both of these changes were due to
[GUACAMOLE-292](https://issues.apache.org/jira/browse/GUACAMOLE-292), part of
0.9.13-incubating.
---
[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r239272644
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
Ping @mike-jumper - just wanted to verify this and then I think I can push
forward with merging this PR.
---
[GitHub] guacamole-client pull request #343: GUACAMOLE-526: Correct redirect issue wi...
Github user asfgit closed the pull request at: https://github.com/apache/guacamole-client/pull/343 ---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user andrzejdoro commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/205#discussion_r239014858 --- Diff: src/guacd/conf-file.h --- @@ -24,6 +24,11 @@ #include "conf.h" +/** + * Creates a new configuration structure, filled with default settings. + */ +guacd_config* guacd_conf_create(); + --- End diff -- After changes, guacd_config creation is independent of parsing configuration files. Should I put this function here or in `conf.h` (and implementation in **new** `conf.c` file)? ---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user andrzejdoro commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/205#discussion_r239016330 --- Diff: src/guacd/conf-file.h --- @@ -32,10 +37,17 @@ int guacd_conf_parse_file(guacd_config* conf, int fd); --- End diff -- What about hiding `guacd_conf_parse_file()` by making it static? Now, `guacd_conf_load` provides this functionality on higher level. ---
[GitHub] guacamole-server pull request #205: GUACAMOLE-667: Enable loading configurat...
Github user andrzejdoro commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/205#discussion_r238983038
--- Diff: src/guacd/conf-args.c ---
@@ -122,3 +128,15 @@ int guacd_conf_parse_args(guacd_config* config, int
argc, char** argv) {
}
+const char* guacd_conf_get_path(int argc, char** argv) {
+
+/* Find -c option */
+int i;
+for(i=1; i I really don't like the idea of ignoring `-c` initially and manually
re-parsing the arguments later. There has to be a better way.
More precisely, it's the other way round: at first we manually search for
`-c` (then load config) and then we parse all the arguments. Nevertheless, I'm
not happy with this solution either, but it seems that _getopt()_ is not
helping.
> What about parsing any file specified with `-c` immediately upon reading
that `-c`?
That's a nice solution! I'm not sure whether it's quite common behaviour,
but we can try it. :+1:
---
