From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:8:6d80::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id WIUKLOHXgWA0IgEAgWs5BA (envelope-from ) for ; Thu, 22 Apr 2021 22:09:05 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id 4EWeJ+HXgWDsewAA1q6Kng (envelope-from ) for ; Thu, 22 Apr 2021 20:09:05 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 569FA210FC for ; Thu, 22 Apr 2021 22:09:05 +0200 (CEST) Received: from localhost ([::1]:44444 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lZfdI-0006Yd-CG for larch@yhetil.org; Thu, 22 Apr 2021 16:09:04 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59006) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lZfI3-0002nu-Iz for guix-devel@gnu.org; Thu, 22 Apr 2021 15:47:07 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:50271) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lZfHz-00012x-0H for guix-devel@gnu.org; Thu, 22 Apr 2021 15:47:07 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id EEBBE27BC7C; Thu, 22 Apr 2021 20:46:59 +0100 (BST) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id f273beee; Thu, 22 Apr 2021 19:46:59 +0000 (UTC) References: <79d3d2e5c1386b1e162f1ba8380562720131856d.camel@telenet.be> <87tuo7xljp.fsf@cbaines.net> <87lf9ixz5j.fsf@cbaines.net> <87wnsziing.fsf@cbaines.net> <87mttuhyoq.fsf@cbaines.net> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: Canan Talayhan Subject: Re: [Outreachy] - Guix Data Service - Set a more informative page title In-reply-to: Date: Thu, 22 Apr 2021 20:46:57 +0100 Message-ID: <878s5ahzke.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: guix-devel@gnu.org Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1619122145; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=WPk4bPy4TvNrGH+koEdDWw4cwqd5Q6gxA/u9B/0qJhA=; b=EvM4D4uQZTThMpA7x4dvxfkCP09R0S30bohzJ6GCI1BgJAq0DTDLzrn6YsmBkKV1uHXXl4 LYqx6jEQc8tQBgNSR7CZCxwPi7wV9JswvH3FdbLo/hEveRIBseijF3irIUEQOMC+b0uITC iKj08FF0cv/aS35i1j5Oa0KU+zE9soDfUShlG3lrY2ndRszV4m3Y8md6l19owNc0H+ktbi td2X0iqw55Ps+/pknjVuvC3EX85wZPXVNeXL4xLJv+c3oQWTzAjTG8vimRqLkhYvXZbnKp EGRFM1KYLZ+VOBjTKXdyRjfpJ74z66uRggs1O1JJPB//esZK623ST2p9pgTkUg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1619122145; a=rsa-sha256; cv=none; b=oMybqWJwCHgaVZl8pUf/xt5M+1qTT/JQdv9a3SaaD+U2lAaEs+W1KRfw+dEbH2O/Ixw70y 47UB7l0zUCRxYRB0r+7YMKpKM+FKAgiiG33dsC6b0IhGP6T2NIUlIfbhW5J4HkHjg7fN4B VCfRapTJk22K8y0uoUg3LBMj6GP+cPhqQ68XqDBLAG+zpA8FIap7l5tL4W9YJSW89vcCkQ IhH2XpI/BfHWPRShw5/gfGQFZksV23+gEhcENP5bqJFE+1gPLyRAMqJm5WjtUBpxqI0Wmq TuXUot8GWD0K6L6td7nPdcjDqunTlJLTFtusNpAx1IZ2nXBzSpMWc7wtaxirDg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Spam-Score: -4.54 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: 569FA210FC X-Spam-Score: -4.54 X-Migadu-Scanner: scn0.migadu.com X-TUID: 5VhL8MM5+JcE --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Canan Talayhan writes: > I've missed it unintentionally. I've not touched the "@" sign this time. = :) > >>> + (define page-header "Comparing") > >> + > >> (layout > >> + #:title > >> + (string-append page-header " " (string-take base-commit 8) " and " > >> + (string-take target-commit 8)) > >> #:body > >> `(,(header) > >> (div > >> @@ -107,7 +112,7 @@ > >> (@ (class "col-sm-7")) > >> ,@(if invalid-query? > >> `((h1 "Compare")) > >> - `((h1 "Comparing " > >> + `((h1 ,page-header ," " > >> (a (@ (href ,(string-append "/revision/" base-commit))) > >> (samp ,(string-take base-commit 8) "=E2=80=A6")) > >> " and " > > I think I misunderstood that comment. >>> There's a couple of things here. I'd be tempted not to use a variable >>> for "Comparing", it's not really the page header, as that's more >>> complicated, so I think I'd just use the string in both places. > > Now, I've fixed it this way. I hope this version is good. > > > > > > > > > > > > > > > > > > > *(layout #:title (string-append "Comparing " (string-take base-commit > 8) " and " (string-take target-commit 8)) #:body `(,(header) > (div (@ (class "container")) (div (@ (class "row")) > (div (@ (class "col-sm-7")) ,@(if invalid-query? > `((h1 "Compare")) `((h1 "Comparing " (a > (@ (href ,(string-append "/revision/" base-commit))) > (samp ,(string-take base-commit 8) "=E2=80=A6")) " an= d "* > I think your email came through a bit garbled, anyway, I think there's still an issue with the title here. > On Mon, Apr 19, 2021 at 10:16 PM Christopher Baines > wrote: > >> >> Canan Talayhan writes: >> >> > Thanks for your quick response. >> > >> >>Why's the @ being removed here? >> > It interprets like an HTML code when I use the page-header like >> > `,page-header, so I removed it. According to your comment, I reverted >> > to the original version. >> > >> > " 'GET repository..." which includes package/package-name in the URL >> > has not the best titles since I couldn't test them because of the >> > error that I've mentioned. >> > I'm open to suggestions. >> > >> > Could you please re-review the patch that contains all the >> > modifications you've mentioned in the previous message? >> >> I've had another look, see my comments below. >> >> > On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines >> wrote: >> >> >> >> >> >> Canan Talayhan writes: >> >> >> >> > I've updated the patch that contains all the suggestions. I think t= he >> patch >> >> > is ready to merge. >> >> > >> >> > One thing that I would like to ask you about the package and >> package-name >> >> > in web/repository/controller.scm. >> >> > >> >> > When I test the URL below I'm getting this error. ( >> >> > https://pastebin.ubuntu.com/p/HdKShmKqH7/) >> >> > >> >> > - ('GET "repository" repository-id "branch" branch-name "package" >> >> > package-name) -> >> >> > http://localhost:8765/repository/1/branch/master/package/emacs >> >> > >> >> > What do you think? BTW it's accessible on the official server. >> >> > >> >> > - >> https://data.guix.gnu.org/repository/1/branch/master/package/emacs/ >> >> >> >> Hmm, this could possibly be due to an issue with the small dump of the >> >> database. >> >> >> >> > Could you please review the patch attached? >> >> > I'm very excited to make my first FOSS contribution. :) >> >> >> >> I've had a look though, and I have some more comments: >> >> >> >> diff --git a/guix-data-service/web/compare/html.scm >> b/guix-data-service/web/compare/html.scm >> >> index 5b5fe0a..170fb12 100644 >> >> --- a/guix-data-service/web/compare/html.scm >> >> +++ b/guix-data-service/web/compare/html.scm >> >> @@ -96,7 +96,12 @@ >> >> (unless invalid-query? >> >> (query-parameters->string query-parameters))) >> >> >> >> + (define page-header "Comparing") >> >> + >> >> (layout >> >> + #:title >> >> + (string-append page-header " " (string-take base-commit 8) " an= d " >> >> + (string-take target-commit 8)) >> >> #:body >> >> `(,(header) >> >> (div >> >> @@ -107,7 +112,7 @@ >> >> (@ (class "col-sm-7")) >> >> ,@(if invalid-query? >> >> `((h1 "Compare")) >> >> - `((h1 "Comparing " >> >> + `((h1 ,page-header ," " >> >> (a (@ (href ,(string-append "/revision/" >> base-commit))) >> >> (samp ,(string-take base-commit 8) "=E2=80= =A6")) >> >> " and " >> >> >> >> There's a couple of things here. I'd be tempted not to use a variable >> >> for "Comparing", it's not really the page header, as that's more >> >> complicated, so I think I'd just use the string in both places. >> >> >> >> Second thing, the (if invalid-query? bit when constructing the h1 >> >> element is important. The query parameters being invalid could mean >> >> anything from the form just hasn't been filled in, to the value isn't >> >> actually a commit hash, but something else, maybe some HTML/JavaScript >> >> that is malicious and shouldn't be included in the page. A similar >> >> approach probably needs taking for the title. >> >> This stuff above still looks the same to me, although part of my >> analysis was wrong, as the type of an invalid parameter is a record, so >> the page just breaks if the parameters are invalid (which I guses is >> better than what I was describing). >> >> Anyway, I think this still needs fixing. These are my relevant comments from before, you should be able to see the error yourself if you go to /compare locally, does this page work for you? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmCB0rFfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XfBLRAAlsmVh2mJfXFh37wHGsfswvJPkQwpoXyM BHw38hDby5SgTaIOA/CiWasgrSj/Poi0ZvBquu7rE0T8f4W2NL63qTXBkL/wPskq YQcMI17fS8Ms5P0tJaOcS/GkwiZnLRrJ0karxdqvQzl6ldICRZ+FFn3aZtdO2US+ Lu1PcFXAF0709lIZWxzI3Y7q6GB+kuYoZWpbrFqSRMeuYYVO1cudH6Z2ZLOUR6wz s0b2VGItcvBP3EE5wnGRBQo+2d/nLjjfvHtkr8jCTLdRuLlCNQUULwBJ69bFS5/q bOmY5u+94Ov3UXza35b4cjODEaWOeGpaZT3haUWa4nTMVvAn97EkL+2s+k1dzIIH fEnC2KWjPHceeJPojxjRv/hvl9sYizQgADQK/NtYxJj6prkIed6NGIIpBPzJXYYh oy3T8kCv5EJZicv/wnu8yaVFoDzQ7WKDXZBUIp/91YHVa3Yils4cTSfG8EQosCio s2y+iVzrP2L+18pygEeY/Om9NVN+pJI96cOu+aybktngctcyqa/bh6AHRRFHCeQ5 NCYnCxFtooMP6hBx0zWR+87xQ/JmtVEo9fZ8Ji0f6jGy95nVJWi9Bg6BdjnMAQqY WnddlxbfWS6FUe5FfQjwxm+TRqElCwpsi+5++uCiQabWY9n3qVT5rHNA5iX1SwN0 K89NcW1UOvI= =uvQC -----END PGP SIGNATURE----- --=-=-=--